From 9d93188ce957e4cc628a5c3ea355ea1d160b6229 Mon Sep 17 00:00:00 2001 From: Aly Raffauf Date: Mon, 10 Aug 2026 20:05:35 -0400 Subject: [PATCH] feat: guide apply conflict resolution --- README.md | 8 +- docs/reconciliation.md | 16 ++-- integration/aliases_test.go | 4 +- integration/apply_test.go | 4 +- integration/hooks_test.go | 2 +- internal/application/apply/decisions.go | 97 ++++++++++++++------ internal/application/apply/decisions_test.go | 31 +++---- internal/application/apply/evaluate.go | 44 ++++----- internal/application/apply/service.go | 23 ++++- internal/application/apply/types.go | 41 +++++++-- internal/bootstrap/applications.go | 1 + internal/cli/apply.go | 3 + internal/cli/apply_test.go | 13 +++ internal/cli/prompt.go | 93 ++++++++++++++----- internal/cli/prompt_test.go | 27 ++++-- internal/reconcile/decisions.go | 25 +---- internal/reconcile/decisions_test.go | 24 ++--- internal/reconcile/types.go | 3 +- internal/reconcile/types_test.go | 8 +- 19 files changed, 303 insertions(+), 164 deletions(-) diff --git a/README.md b/README.md index 38a4bcc..918e244 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ configuration directory—in `_routes.toml`. | `cattery version` | Print build information. | Global options are `--repo PATH` and `--verbose`. `apply` supports `--dry-run`, -`--non-interactive`, `--no-hooks`, and `--skip-secrets`; `add` supports `--group`, `--platform`, +`--non-interactive`, `--no-hooks`, `--skip-secrets`, and `--force`; `add` supports `--group`, `--platform`, `--secret`, and `--dry-run`; `forget` supports `--dry-run` and requires `--yes` to remove repository sources. Secret lifecycle commands accept repeatable `--source REPOSITORY_PATH` selectors; `secrets reencrypt` previews by default @@ -93,7 +93,13 @@ and requires `--yes` to replace encrypted sources. your files. - If the local state database is lost, equal files are adopted again and different files require a decision. +- Interactive conflicts show a safe preview automatically. Enter `r` (or + `repository`) to use the repository version, `s`/`skip` to leave that item, + or `a`/`abort` to stop. A final `y` confirmation is required before writes. +- `apply --force` chooses the repository version for every selected conflict. + It does not bypass target validation, SOPS, atomic writes, or other safety checks. - A non-interactive session stops before any change that would need a prompt. + Use `cattery add` to explicitly adopt local content into the repository. ## Secrets and hooks diff --git a/docs/reconciliation.md b/docs/reconciliation.md index 0f9cec2..ad5e0d6 100644 --- a/docs/reconciliation.md +++ b/docs/reconciliation.md @@ -38,15 +38,13 @@ choose between silently replacing local work and abandoning the repository. Before any write, interactive `apply` gathers every decision it needs: -```text -[d]iff -[o]verwrite target from repository -[s]kip -[a]bort -``` - -`diff` shows a unified diff for ordinary files and then asks again. Secrets -never expose plaintext in a diff. `overwrite` authorizes that one target; +Ordinary files show a safe unified diff automatically. Secrets only report that +encrypted content differs and never expose plaintext. Choose `r` (or +`repository`) to authorize that target, `s`/`skip` to leave it, or `a`/`abort` +to stop. After all choices, `apply` shows a resolution summary and requires +`y` before hooks or writes begin. `apply --force` selects repository for every +selected conflict without disabling validation or write safeguards. To adopt +local content into the repository, use `cattery add` instead. `skip` leaves it unresolved while other safe work can continue; `abort` stops before hooks or writes begin. diff --git a/integration/aliases_test.go b/integration/aliases_test.go index effa163..3d59544 100644 --- a/integration/aliases_test.go +++ b/integration/aliases_test.go @@ -93,7 +93,7 @@ func testAliasWrong(t *testing.T) { if err := os.Symlink("elsewhere", filepath.Join(env.home, "bin", "tool")); err != nil { t.Fatal(err) } - result := env.runPty(t, []string{"overwrite"}, "apply") + result := env.runPty(t, []string{"overwrite", "y"}, "apply") if result.Code != 0 { t.Fatalf("apply: code=%d stderr=%q", result.Code, result.Stderr) } @@ -111,7 +111,7 @@ func testAliasOccupied(t *testing.T) { t.Fatal(err) } writeFile(t, filepath.Join(env.home, "bin", "tool"), []byte("intruder")) - result := env.runPty(t, []string{"overwrite"}, "apply") + result := env.runPty(t, []string{"overwrite", "y"}, "apply") if result.Code != 0 { t.Fatalf("apply: code=%d stderr=%q", result.Code, result.Stderr) } diff --git a/integration/apply_test.go b/integration/apply_test.go index 01a80bd..94ef999 100644 --- a/integration/apply_test.go +++ b/integration/apply_test.go @@ -127,7 +127,7 @@ func testExecApplyOverwrite(t *testing.T) { t.Fatalf("first apply: %+v", result) } writeFile(t, filepath.Join(env.home, ".config", "app"), []byte("drifted")) - result := env.runPty(t, []string{"overwrite"}, "apply") + result := env.runPty(t, []string{"overwrite", "y"}, "apply") if result.Code != 0 { t.Fatalf("apply: code=%d stderr=%q", result.Code, result.Stderr) } @@ -144,7 +144,7 @@ func testExecApplySkip(t *testing.T) { t.Fatalf("first apply: %+v", result) } writeFile(t, filepath.Join(env.home, ".config", "app"), []byte("drifted")) - result := env.runPty(t, []string{"skip"}, "apply") + result := env.runPty(t, []string{"skip", "y"}, "apply") if result.Code != 2 { t.Fatalf("code = %d, want 2 after a skip", result.Code) } diff --git a/integration/hooks_test.go b/integration/hooks_test.go index 1863fec..cbe99f0 100644 --- a/integration/hooks_test.go +++ b/integration/hooks_test.go @@ -103,7 +103,7 @@ func testHooksResultEnv(t *testing.T) { env.source(t, ".config/app", "v1") installHooks(t, env) writeFile(t, filepath.Join(env.home, ".config", "app"), []byte("drifted")) - result := env.runPty(t, []string{"skip"}, "apply") + result := env.runPty(t, []string{"skip", "y"}, "apply") if result.Code != 2 { t.Fatalf("code = %d, want 2 after a skip", result.Code) } diff --git a/internal/application/apply/decisions.go b/internal/application/apply/decisions.go index a5c4f02..524ad5b 100644 --- a/internal/application/apply/decisions.go +++ b/internal/application/apply/decisions.go @@ -3,6 +3,7 @@ package apply import ( "context" + "github.com/alyraffauf/cattery/internal/deployment" "github.com/alyraffauf/cattery/internal/failure" "github.com/alyraffauf/cattery/internal/reconcile" ) @@ -31,10 +32,12 @@ func (c CollectedDecisions) Specs() []reconcile.DecisionSpec { // CollectDecisions resolves every candidate that requires an explicit // decision, in bytewise target-path order, and validates each response -// before any hook or mutation. An abort answer stops -// the whole apply; a diff answer re-requests the adapter, which shows the -// safe difference and asks again. +// before any hook or mutation. An abort answer stops the whole apply. func (service *Service) CollectDecisions(ctx context.Context, candidates Candidates) (CollectedDecisions, error) { + return service.collectDecisions(ctx, Request{}, candidates) +} + +func (service *Service) collectDecisions(ctx context.Context, applyRequest Request, candidates Candidates) (CollectedDecisions, error) { if err := ctx.Err(); err != nil { return CollectedDecisions{}, err } @@ -45,6 +48,14 @@ func (service *Service) CollectDecisions(ctx context.Context, candidates Candida ordered := reconcile.OrderedDecisionSpecs(specs) decisions := make([]ResolvedDecision, 0, len(ordered)) for _, spec := range ordered { + if applyRequest.Force { + request, err := service.decisionRequest(spec, candidates) + if err != nil { + return CollectedDecisions{}, err + } + decisions = append(decisions, ResolvedDecision{request: request, response: DecisionResponse{Choice: ChoiceOverwrite}}) + continue + } decision, err := service.collectOne(ctx, spec, candidates) if err != nil { return CollectedDecisions{}, err @@ -57,14 +68,11 @@ func (service *Service) CollectDecisions(ctx context.Context, candidates Candida // collectOne projects one spec into a request, resolves it, and validates // the response before any hook or mutation. func (service *Service) collectOne(ctx context.Context, spec reconcile.DecisionSpec, candidates Candidates) (ResolvedDecision, error) { - request, err := NewDecisionRequest(DecisionRequestInput{ - TargetPath: spec.TargetPath(), - Choices: projectChoices(spec.AllChoices()), - }) + request, err := service.decisionRequest(spec, candidates) if err != nil { return ResolvedDecision{}, failure.New(failure.InvalidInput, "apply: project decision request", err) } - response, err := service.resolveRepeatedly(ctx, request, service.differenceProvider(candidates)) + response, err := service.resolveOnce(decisionResolution{context: ctx, request: request, difference: service.differenceProvider(candidates)}) if err != nil { return ResolvedDecision{}, err } @@ -74,6 +82,59 @@ func (service *Service) collectOne(ctx context.Context, spec reconcile.DecisionS return ResolvedDecision{request: request, response: response}, nil } +func (service *Service) decisionRequest(spec reconcile.DecisionSpec, candidates Candidates) (DecisionRequest, error) { + candidate := candidateFor(spec.TargetPath(), candidates) + input := DecisionRequestInput{TargetPath: spec.TargetPath(), Choices: projectChoices(spec.AllChoices()), Kind: decisionKind(candidate), Reason: decisionReason(spec.Reason())} + if candidate.record.Entry == reconcile.PlanEntryAlias { + input.ExpectedLink = candidate.record.Alias.CanonicalTargetRelativePath + input.CurrentLink = candidate.record.Target.Payload() + } + request, err := NewDecisionRequest(input) + if err != nil { + return DecisionRequest{}, failure.New(failure.InvalidInput, "apply: project decision request", err) + } + return request, nil +} + +func candidateFor(target string, candidates Candidates) Candidate { + for _, candidate := range candidates.All() { + if candidate.record.TargetPath == target { + return candidate + } + } + return Candidate{} +} + +func decisionKind(candidate Candidate) string { + if candidate.record.Entry == reconcile.PlanEntryAlias { + return "alias" + } + if candidate.record.File.Kind == deployment.FileSecret { + return "secret" + } + return "file" +} + +func decisionReason(reason reconcile.Reason) string { + switch reason { + case reconcile.ReasonConflict: + return "both the repository and local target changed" + case reconcile.ReasonTargetDrift: + return "the local target differs from its recorded baseline" + case reconcile.ReasonUnbaselinedDiffer: + return "the target differs and has no recorded baseline" + case reconcile.ReasonUnexpectedTargetType: + return "the target has an unsupported type" + case reconcile.ReasonAliasWrong: + return "the link points somewhere else" + case reconcile.ReasonAliasOccupied: + return "the alias path is occupied" + case reconcile.ReasonRepresentationDrift: + return "the target representation differs from the repository" + } + return "the target requires a decision" +} + // decisionSpecs collects the frozen specs of every candidate that requires // an explicit decision, in evaluation order. func decisionSpecs(candidates Candidates) ([]reconcile.DecisionSpec, error) { @@ -130,30 +191,10 @@ func projectChoice(choice reconcile.DecisionChoice) DecisionChoice { return ChoiceSkip case reconcile.ChoiceAbort: return ChoiceAbort - case reconcile.ChoiceDiff: - return ChoiceDiff } return "" } -// resolveRepeatedly asks the resolver until it returns a final choice, -// re-requesting when it answers diff so the adapter can show the safe -// difference and ask again. -func (service *Service) resolveRepeatedly(ctx context.Context, request DecisionRequest, difference DifferenceProvider) (DecisionResponse, error) { - for { - if err := ctx.Err(); err != nil { - return DecisionResponse{}, err - } - response, err := service.resolveOnce(decisionResolution{context: ctx, request: request, difference: difference}) - if err != nil { - return DecisionResponse{}, err - } - if response.Choice != ChoiceDiff { - return response, nil - } - } -} - // resolveOnce asks the resolver once and validates its response against the // allowed choices of the request. type decisionResolution struct { diff --git a/internal/application/apply/decisions_test.go b/internal/application/apply/decisions_test.go index eb648af..8a9b108 100644 --- a/internal/application/apply/decisions_test.go +++ b/internal/application/apply/decisions_test.go @@ -17,7 +17,7 @@ func TestApplyDecisionCollection(t *testing.T) { {"abort stops the apply", testDecisionAbort}, {"skip is collected", testDecisionSkip}, {"overwrite is collected", testDecisionOverwrite}, - {"diff re-requests", testDecisionDiff}, + {"force chooses repository without resolver", testDecisionForce}, {"diff provider receives the candidate", testDecisionDifferenceProvider}, {"invalid response", testDecisionInvalid}, {"resolver errors propagate", testDecisionResolverError}, @@ -28,6 +28,19 @@ func TestApplyDecisionCollection(t *testing.T) { } } +func testDecisionForce(t *testing.T) { + service, candidates := decisionFixture(t, "a.conf") + resolver := &resolverFake{err: fmt.Errorf("resolver must not run")} + service.resolver = resolver + collected, err := service.collectDecisions(context.Background(), Request{Force: true}, candidates) + if err != nil { + t.Fatalf("collect: %v", err) + } + if len(resolver.requests) != 0 || collected.All()[0].response.Choice != ChoiceOverwrite { + t.Fatalf("force must synthesize one repository choice without prompting: %+v", collected.All()) + } +} + // resolverFake returns queued responses and records the requests. type resolverFake struct { responses []DecisionResponse @@ -117,22 +130,6 @@ func testDecisionOverwrite(t *testing.T) { } } -func testDecisionDiff(t *testing.T) { - resolver := &resolverFake{responses: []DecisionResponse{{Choice: ChoiceDiff}, {Choice: ChoiceSkip}}} - service, candidates := decisionFixture(t, "a.conf") - service.resolver = resolver - collected, err := service.CollectDecisions(context.Background(), candidates) - if err != nil { - t.Fatalf("collect: %v", err) - } - if len(resolver.requests) != 2 { - t.Fatalf("diff answer must re-request, requests = %v", resolver.requests) - } - if collected.All()[0].response.Choice != ChoiceSkip { - t.Fatalf("final decision = %+v, want skip", collected.All()[0]) - } -} - func testDecisionDifferenceProvider(t *testing.T) { resolver := &resolverFake{responses: []DecisionResponse{{Choice: ChoiceSkip}}} service, candidates := decisionFixture(t, "a.conf") diff --git a/internal/application/apply/evaluate.go b/internal/application/apply/evaluate.go index 6aded47..a8264a8 100644 --- a/internal/application/apply/evaluate.go +++ b/internal/application/apply/evaluate.go @@ -11,17 +11,18 @@ import ( // Service evaluates an apply request before decisions, hooks, or mutations. type Service struct { - evaluator *evaluation.Service - state StateReader - secrets *secrets.Client - client SecretClient - replacer AtomicReplacer - baselines BaselineStore - transitions TransitionStore - retirements RetirementStore - hooks HookExecutor - probe DependencyProbe - resolver DecisionResolver + evaluator *evaluation.Service + state StateReader + secrets *secrets.Client + client SecretClient + replacer AtomicReplacer + baselines BaselineStore + transitions TransitionStore + retirements RetirementStore + hooks HookExecutor + probe DependencyProbe + resolver DecisionResolver + confirmation Confirmation } // NewService constructs the apply service over its injected ports. @@ -37,16 +38,17 @@ func NewService(dependencies Dependencies) *Service { CommandLabel: "apply", IncludeUnmanagedTargetDigest: true, }), - state: dependencies.State, - secrets: dependencies.Secrets, - client: dependencies.Client, - replacer: dependencies.Replacer, - baselines: dependencies.Baselines, - transitions: dependencies.Transitions, - retirements: dependencies.Retirements, - hooks: dependencies.Hooks, - probe: dependencies.Probe, - resolver: dependencies.Resolver, + state: dependencies.State, + secrets: dependencies.Secrets, + client: dependencies.Client, + replacer: dependencies.Replacer, + baselines: dependencies.Baselines, + transitions: dependencies.Transitions, + retirements: dependencies.Retirements, + hooks: dependencies.Hooks, + probe: dependencies.Probe, + resolver: dependencies.Resolver, + confirmation: dependencies.Confirmation, } } diff --git a/internal/application/apply/service.go b/internal/application/apply/service.go index a83a7a0..625baad 100644 --- a/internal/application/apply/service.go +++ b/internal/application/apply/service.go @@ -23,10 +23,13 @@ func (service *Service) Apply(ctx context.Context, request Request) (Result, err if err := service.Preflight(ctx, candidates); err != nil { return Result{}, err } - decisions, err := service.CollectDecisions(ctx, candidates) + decisions, err := service.collectDecisions(ctx, request, candidates) if err != nil { return Result{}, err } + if err := service.confirmDecisions(ctx, request, decisions); err != nil { + return Result{}, err + } plan, err := service.Prepare(ctx, PrepareInput{Request: request, Candidates: candidates, Decisions: decisions}) if err != nil { return Result{}, err @@ -37,6 +40,24 @@ func (service *Service) Apply(ctx context.Context, request Request) (Result, err return service.execute(ctx, executeInput{request: request, plan: plan, candidates: candidates}) } +func (service *Service) confirmDecisions(ctx context.Context, request Request, decisions CollectedDecisions) error { + if request.Force || len(decisions.All()) == 0 || service.confirmation == nil { + return nil + } + resolutions := make([]Resolution, 0, len(decisions.All())) + for _, decision := range decisions.All() { + resolutions = append(resolutions, Resolution{Request: decision.request, Choice: decision.response.Choice}) + } + confirmed, err := service.confirmation.Confirm(ctx, resolutions) + if err != nil { + return err + } + if !confirmed { + return failure.New(failure.Difference, "apply: resolution review declined", nil) + } + return nil +} + // dryOutcome freezes the dry-run result, reporting pending changes as a // difference so the CLI exits 2. func (service *Service) dryOutcome(plan PreparedPlan) (Result, error) { diff --git a/internal/application/apply/types.go b/internal/application/apply/types.go index 7662e43..93c554d 100644 --- a/internal/application/apply/types.go +++ b/internal/application/apply/types.go @@ -36,6 +36,7 @@ type Dependencies struct { Hooks HookExecutor Probe DependencyProbe Resolver DecisionResolver + Confirmation Confirmation ProtectedTrees []string Platform string } @@ -105,6 +106,18 @@ type DecisionResolver interface { Resolve(context.Context, DecisionRequest) (DecisionResponse, error) } +// Confirmation approves an interactive resolution session after every +// per-target choice has been collected and before any hook or write runs. +type Confirmation interface { + Confirm(context.Context, []Resolution) (bool, error) +} + +// Resolution is one safe, CLI-facing final-review row. +type Resolution struct { + Request DecisionRequest + Choice DecisionChoice +} + // DifferenceProvider renders one safe difference for the current apply // candidate. It returns false when the target cannot produce a difference. type DifferenceProvider func(context.Context, string) (SafeDifference, bool) @@ -131,6 +144,7 @@ type Request struct { NonInteractive bool NoHooks bool SkipSecrets bool + Force bool } // DecisionChoice is the application-owned choice vocabulary one prompt may @@ -144,22 +158,28 @@ const ( ChoiceSkip DecisionChoice = "skip" // ChoiceAbort stops the whole apply. ChoiceAbort DecisionChoice = "abort" - // ChoiceDiff shows the safe source/target difference first. - ChoiceDiff DecisionChoice = "diff" ) // DecisionRequest is the frozen prompt request the CLI resolves: the // HOME-relative target path and the allowed choices projected from the // reconcile decision spec. Only application-owned types appear. type DecisionRequest struct { - targetPath string - choices []DecisionChoice + targetPath string + choices []DecisionChoice + kind string + reason string + expectedLink string + currentLink string } // DecisionRequestInput carries the projection fields of one prompt request. type DecisionRequestInput struct { - TargetPath string - Choices []DecisionChoice + TargetPath string + Choices []DecisionChoice + Kind string + Reason string + ExpectedLink string + CurrentLink string } // NewDecisionRequest validates candidate field-by-field and freezes it. The @@ -176,12 +196,12 @@ func NewDecisionRequest(candidate DecisionRequestInput) (DecisionRequest, error) return DecisionRequest{}, fmt.Errorf("apply: decision request for %q has invalid choice %q", candidate.TargetPath, choice) } } - return DecisionRequest{targetPath: candidate.TargetPath, choices: append([]DecisionChoice(nil), candidate.Choices...)}, nil + return DecisionRequest{targetPath: candidate.TargetPath, choices: append([]DecisionChoice(nil), candidate.Choices...), kind: candidate.Kind, reason: candidate.Reason, expectedLink: candidate.ExpectedLink, currentLink: candidate.CurrentLink}, nil } func validChoice(choice DecisionChoice) bool { switch choice { - case ChoiceOverwrite, ChoiceSkip, ChoiceAbort, ChoiceDiff: + case ChoiceOverwrite, ChoiceSkip, ChoiceAbort: return true } return false @@ -195,6 +215,11 @@ func (r DecisionRequest) Choices() []DecisionChoice { return append([]DecisionChoice(nil), r.choices...) } +func (r DecisionRequest) Kind() string { return r.kind } +func (r DecisionRequest) Reason() string { return r.reason } +func (r DecisionRequest) ExpectedLink() string { return r.expectedLink } +func (r DecisionRequest) CurrentLink() string { return r.currentLink } + // DecisionResponse is the frozen prompt response the CLI returns. type DecisionResponse struct { Choice DecisionChoice diff --git a/internal/bootstrap/applications.go b/internal/bootstrap/applications.go index fd9b540..ea5961f 100644 --- a/internal/bootstrap/applications.go +++ b/internal/bootstrap/applications.go @@ -153,6 +153,7 @@ func buildApply(input ApplicationsInput, shared shared) *apply.Service { Hooks: hookAdapter{}, Probe: &probeAdapter{}, Resolver: shared.prompt, + Confirmation: shared.prompt, ProtectedTrees: input.Protected, Platform: string(input.Platform), }) diff --git a/internal/cli/apply.go b/internal/cli/apply.go index 6a7a8c5..ee800dd 100644 --- a/internal/cli/apply.go +++ b/internal/cli/apply.go @@ -38,6 +38,7 @@ func newApplyCommand(service ApplyService, runtime Runtime, options *Options) *c command.Flags().Bool("non-interactive", false, "refuse unresolved decisions") command.Flags().Bool("no-hooks", false, "skip trusted hooks") command.Flags().Bool("skip-secrets", false, "skip encrypted secret targets") + command.Flags().Bool("force", false, "use repository versions for all selected conflicts") return command } @@ -57,6 +58,7 @@ func applyRequest(command *cobra.Command, input applyInput) apply.Request { nonInteractive, _ := command.Flags().GetBool("non-interactive") noHooks, _ := command.Flags().GetBool("no-hooks") skipSecrets, _ := command.Flags().GetBool("skip-secrets") + force, _ := command.Flags().GetBool("force") return apply.Request{ Repository: applyRepository(options, input.runtime), Groups: append([]string(nil), input.groups...), @@ -64,6 +66,7 @@ func applyRequest(command *cobra.Command, input applyInput) apply.Request { NonInteractive: nonInteractive, NoHooks: noHooks, SkipSecrets: skipSecrets, + Force: force, } } diff --git a/internal/cli/apply_test.go b/internal/cli/apply_test.go index 67d1379..58ebffa 100644 --- a/internal/cli/apply_test.go +++ b/internal/cli/apply_test.go @@ -18,6 +18,7 @@ func TestApplyCommand(t *testing.T) { {"flags and args mapped", testApplyFlags}, {"one call", testApplyOneCall}, {"dry run flag", testApplyDryRun}, + {"force flag", testApplyForce}, {"partial error joins", testApplyPartial}, {"writer failure", testApplyWriterError}, } @@ -95,6 +96,18 @@ func testApplyDryRun(t *testing.T) { } } +func testApplyForce(t *testing.T) { + service := &applyServiceFake{result: applyResult()} + command, _ := applyFixture(t, service, Options{}) + command.SetArgs([]string{"--force", "apps"}) + if err := command.Execute(); err != nil { + t.Fatalf("run: %v", err) + } + if !service.requests[0].Force { + t.Fatalf("force = false, want true") + } +} + func testApplyPartial(t *testing.T) { service := &applyServiceFake{result: applyResult(), err: failure.New(failure.Operational, "apply: partial write", nil)} command, stdout := applyFixture(t, service, Options{}) diff --git a/internal/cli/prompt.go b/internal/cli/prompt.go index de6da2f..e6d953a 100644 --- a/internal/cli/prompt.go +++ b/internal/cli/prompt.go @@ -26,10 +26,10 @@ type PromptInput struct { // re-prompting on invalid input. It imports only the // apply DTOs and failure categories; no Cobra command or backend adapter. type DecisionPrompt struct { - stdin io.Reader stderr io.Writer isTerminal func(fd int) bool diff func(context.Context, string) (apply.SafeDifference, bool) + scanner *bufio.Scanner } // NewDecisionPrompt builds the interactive resolver over the given input, @@ -40,10 +40,10 @@ func NewDecisionPrompt(input PromptInput) *DecisionPrompt { isTerminal = term.IsTerminal } return &DecisionPrompt{ - stdin: input.Stdin, stderr: input.Stderr, isTerminal: isTerminal, diff: input.Diff, + scanner: bufio.NewScanner(input.Stdin), } } @@ -62,13 +62,15 @@ func (p *DecisionPrompt) resolve(ctx context.Context, request apply.DecisionRequ if p.isTerminal == nil || !p.isTerminal(0) { return apply.DecisionResponse{}, failure.New(failure.Difference, "cli: decisions require an interactive terminal", nil) } - scanner := bufio.NewScanner(p.stdin) path := "$HOME/" + displayPath(request.TargetPath()) + if err := p.renderContext(ctx, path, request, difference); err != nil { + return apply.DecisionResponse{}, err + } for { if err := p.renderPrompt(path, request.Choices()); err != nil { return apply.DecisionResponse{}, err } - answer, err := readAnswer(scanner) + answer, err := readAnswer(p.scanner) if err != nil { return apply.DecisionResponse{}, failure.New(failure.InvalidInput, "cli: EOF before a valid answer", err) } @@ -86,19 +88,10 @@ func (p *DecisionPrompt) resolve(ctx context.Context, request apply.DecisionRequ // renderPrompt writes the choice line of one request to stderr. func (p *DecisionPrompt) renderPrompt(path string, choices []apply.DecisionChoice) error { - _, err := fmt.Fprintf(p.stderr, "%s: %s [%s] ", path, strings.Join(choiceNames(choices), " "), choices[0]) + _, err := fmt.Fprintf(p.stderr, "%s: choose [r]epository, [s]kip, or [a]bort: ", path) return err } -// choiceNames projects the allowed choices into stable lowercase words. -func choiceNames(choices []apply.DecisionChoice) []string { - names := make([]string, 0, len(choices)) - for _, choice := range choices { - names = append(names, string(choice)) - } - return names -} - // readAnswer reads one trimmed answer line. func readAnswer(scanner *bufio.Scanner) (string, error) { if !scanner.Scan() { @@ -121,25 +114,79 @@ type promptAnswer struct { func (p *DecisionPrompt) answer(input promptAnswer) (apply.DecisionResponse, bool, error) { if input.answer == "" { - return apply.DecisionResponse{Choice: input.request.Choices()[0]}, true, nil + _, err := fmt.Fprintln(p.stderr, "choose r, s, or a") + return apply.DecisionResponse{}, false, err } - choice := apply.DecisionChoice(input.answer) + choice := shortChoice(input.answer) for _, allowed := range input.request.Choices() { if choice != allowed { continue } - if choice != apply.ChoiceDiff { - return apply.DecisionResponse{Choice: choice}, true, nil - } - if err := p.renderDifference(input.context, input.request.TargetPath(), input.difference); err != nil { - return apply.DecisionResponse{}, false, err - } - return apply.DecisionResponse{}, false, nil + return apply.DecisionResponse{Choice: choice}, true, nil } _, err := fmt.Fprintf(p.stderr, "invalid answer %q\n", input.answer) return apply.DecisionResponse{}, false, err } +func shortChoice(answer string) apply.DecisionChoice { + switch strings.ToLower(answer) { + case "r", "repository", "overwrite": + return apply.ChoiceOverwrite + case "s", "skip": + return apply.ChoiceSkip + case "a", "abort": + return apply.ChoiceAbort + } + return apply.DecisionChoice(answer) +} + +func (p *DecisionPrompt) renderContext(ctx context.Context, path string, request apply.DecisionRequest, difference apply.DifferenceProvider) error { + if _, err := fmt.Fprintf(p.stderr, "Conflict at %s: %s.\n", path, request.Reason()); err != nil { + return err + } + switch request.Kind() { + case "secret": + _, err := fmt.Fprintln(p.stderr, "Encrypted secret content differs; plaintext is not shown.") + return err + case "alias": + if request.ExpectedLink() != "" { + if _, err := fmt.Fprintf(p.stderr, "Expected link: %s\n", request.ExpectedLink()); err != nil { + return err + } + } + if request.CurrentLink() != "" { + _, err := fmt.Fprintf(p.stderr, "Current link: %s\n", request.CurrentLink()) + return err + } + return nil + default: + return p.renderDifference(ctx, request.TargetPath(), difference) + } +} + +// Confirm presents the final review for an interactive resolution session. +func (p *DecisionPrompt) Confirm(ctx context.Context, resolutions []apply.Resolution) (bool, error) { + if p.isTerminal == nil || !p.isTerminal(0) { + return false, failure.New(failure.Difference, "cli: confirmation requires an interactive terminal", nil) + } + if _, err := fmt.Fprintln(p.stderr, "Resolution summary:"); err != nil { + return false, err + } + for _, resolution := range resolutions { + if _, err := fmt.Fprintf(p.stderr, " %s: $HOME/%s\n", resolution.Choice, displayPath(resolution.Request.TargetPath())); err != nil { + return false, err + } + } + if _, err := fmt.Fprint(p.stderr, "Proceed? [y/N] "); err != nil { + return false, err + } + answer, err := readAnswer(p.scanner) + if err != nil { + return false, failure.New(failure.Difference, "cli: confirmation cancelled", err) + } + return strings.EqualFold(answer, "y") || strings.EqualFold(answer, "yes"), nil +} + // renderDifference displays the safe difference of one target on stderr. func (p *DecisionPrompt) renderDifference(ctx context.Context, target string, difference apply.DifferenceProvider) error { if difference == nil { diff --git a/internal/cli/prompt_test.go b/internal/cli/prompt_test.go index 1b2895b..86abc02 100644 --- a/internal/cli/prompt_test.go +++ b/internal/cli/prompt_test.go @@ -18,10 +18,11 @@ func TestDecisionPrompt(t *testing.T) { {"each choice", testPromptChoices}, {"invalid input reprompts", testPromptInvalid}, {"secret restriction", testPromptSecret}, - {"diff displays and reprompts", testPromptDiff}, + {"diff displays automatically", testPromptDiff}, {"non-terminal refusal", testPromptNonTTY}, {"eof handling", testPromptEOF}, - {"empty defaults", testPromptDefault}, + {"empty is not destructive", testPromptDefault}, + {"final confirmation", testPromptConfirmation}, {"writer errors", testPromptWriterError}, } for _, scenario := range scenarios { @@ -50,7 +51,7 @@ func promptFixture(t *testing.T, answers []string, diff func(context.Context, st func driftRequest() apply.DecisionRequest { request, err := apply.NewDecisionRequest(apply.DecisionRequestInput{ TargetPath: "a.conf", - Choices: []apply.DecisionChoice{apply.ChoiceDiff, apply.ChoiceOverwrite, apply.ChoiceSkip, apply.ChoiceAbort}, + Choices: []apply.DecisionChoice{apply.ChoiceOverwrite, apply.ChoiceSkip, apply.ChoiceAbort}, }) if err != nil { panic(err) @@ -120,7 +121,7 @@ func testPromptDiff(t *testing.T) { provider := func(ctx context.Context, target string) (apply.SafeDifference, bool) { return apply.SafeDifference{Lines: []string{"-old", "+new"}}, true } - prompt, stderr := promptFixture(t, []string{"diff", "overwrite"}, provider) + prompt, stderr := promptFixture(t, []string{"r"}, provider) response, err := prompt.Resolve(context.Background(), driftRequest()) if err != nil { t.Fatalf("resolve: %v", err) @@ -155,12 +156,20 @@ func testPromptEOF(t *testing.T) { func testPromptDefault(t *testing.T) { prompt, _ := promptFixture(t, []string{""}, nil) - response, err := prompt.Resolve(context.Background(), driftRequest()) - if err != nil { - t.Fatalf("resolve: %v", err) + _, err := prompt.Resolve(context.Background(), driftRequest()) + if err == nil || !kindIs(err, failure.InvalidInput) { + t.Fatalf("blank input must not choose a destructive default: %v", err) } - if response.Choice != apply.ChoiceDiff { - t.Fatalf("empty answer must default to the first choice, got %v", response.Choice) +} + +func testPromptConfirmation(t *testing.T) { + prompt, stderr := promptFixture(t, []string{"y"}, nil) + confirmed, err := prompt.Confirm(context.Background(), []apply.Resolution{{Request: driftRequest(), Choice: apply.ChoiceSkip}}) + if err != nil || !confirmed { + t.Fatalf("confirmation = %t, %v", confirmed, err) + } + if !strings.Contains(stderr.String(), "skip: $HOME/a.conf") { + t.Fatalf("summary = %q", stderr.String()) } } diff --git a/internal/reconcile/decisions.go b/internal/reconcile/decisions.go index 7c121fc..8d04e06 100644 --- a/internal/reconcile/decisions.go +++ b/internal/reconcile/decisions.go @@ -20,32 +20,13 @@ func decisionReason(reason Reason) bool { return false } -// diffEligible reports whether a decision may compare source and target -// bytes: only the ordinary file drift rows do. Secrets never expose a diff -// prompt, and an unexpected-type symlink target has no bytes to compare. -func diffEligible(reason Reason, kind deployment.FileKind) bool { - if kind != deployment.FileOrdinary { - return false - } - switch reason { - case ReasonTargetDrift, ReasonConflict, ReasonUnbaselinedDiffer: - return true - } - return false -} - -// AllowedChoices returns the ordered choices one decision prompt may offer -// for the action/reason pair and source kind: every decision offers -// overwrite, skip, and abort, and a diff choice precedes them only for an -// ordinary file decision that compares exact bytes. A pair that does not -// require a decision yields no choices. +// AllowedChoices returns the ordered choices one decision prompt may offer. +// Safe file differences are rendered automatically by the CLI, so every +// decision uses the same repository, skip, and abort vocabulary. func AllowedChoices(action Action, reason Reason, kind deployment.FileKind) []DecisionChoice { if action != ActionNeedsDecision || !decisionReason(reason) { return nil } - if diffEligible(reason, kind) { - return []DecisionChoice{ChoiceDiff, ChoiceOverwrite, ChoiceSkip, ChoiceAbort} - } return []DecisionChoice{ChoiceOverwrite, ChoiceSkip, ChoiceAbort} } diff --git a/internal/reconcile/decisions_test.go b/internal/reconcile/decisions_test.go index 62cddc1..c2bae56 100644 --- a/internal/reconcile/decisions_test.go +++ b/internal/reconcile/decisions_test.go @@ -13,8 +13,7 @@ const ( ) var ( - overwriteSkipAbort = []DecisionChoice{ChoiceOverwrite, ChoiceSkip, ChoiceAbort} - diffOverwriteSkipAbort = []DecisionChoice{ChoiceDiff, ChoiceOverwrite, ChoiceSkip, ChoiceAbort} + overwriteSkipAbort = []DecisionChoice{ChoiceOverwrite, ChoiceSkip, ChoiceAbort} ) // TestDecisionSpecification pins the decision-spec contract: @@ -60,9 +59,9 @@ type eligibilityCase struct { // decision offers overwrite, skip, and abort; diff rides only on ordinary // byte-comparing file rows; automatic and non-decision pairs offer nothing. var decisionEligibilityCases = []eligibilityCase{ - {name: "ordinary drift", action: ActionNeedsDecision, reason: ReasonTargetDrift, kind: deployment.FileOrdinary, want: diffOverwriteSkipAbort}, - {name: "ordinary conflict", action: ActionNeedsDecision, reason: ReasonConflict, kind: deployment.FileOrdinary, want: diffOverwriteSkipAbort}, - {name: "ordinary unbaselined differ", action: ActionNeedsDecision, reason: ReasonUnbaselinedDiffer, kind: deployment.FileOrdinary, want: diffOverwriteSkipAbort}, + {name: "ordinary drift", action: ActionNeedsDecision, reason: ReasonTargetDrift, kind: deployment.FileOrdinary, want: overwriteSkipAbort}, + {name: "ordinary conflict", action: ActionNeedsDecision, reason: ReasonConflict, kind: deployment.FileOrdinary, want: overwriteSkipAbort}, + {name: "ordinary unbaselined differ", action: ActionNeedsDecision, reason: ReasonUnbaselinedDiffer, kind: deployment.FileOrdinary, want: overwriteSkipAbort}, {name: "secret drift", action: ActionNeedsDecision, reason: ReasonTargetDrift, kind: deployment.FileSecret, want: overwriteSkipAbort}, {name: "secret conflict", action: ActionNeedsDecision, reason: ReasonConflict, kind: deployment.FileSecret, want: overwriteSkipAbort}, {name: "secret unbaselined differ", action: ActionNeedsDecision, reason: ReasonUnbaselinedDiffer, kind: deployment.FileSecret, want: overwriteSkipAbort}, @@ -92,10 +91,10 @@ type specCase struct { // classification: only DecisionRequired outcomes produce a spec, carrying // exactly the allowed choices of their action, reason, and source kind. var decisionSpecCases = []specCase{ - {name: "file drift", path: decisionPath, action: ActionNeedsDecision, reason: ReasonTargetDrift, convergence: ConvergenceDecisionRequired, kind: deployment.FileOrdinary, want: diffOverwriteSkipAbort}, + {name: "file drift", path: decisionPath, action: ActionNeedsDecision, reason: ReasonTargetDrift, convergence: ConvergenceDecisionRequired, kind: deployment.FileOrdinary, want: overwriteSkipAbort}, {name: "file drift secret", path: decisionPath, action: ActionNeedsDecision, reason: ReasonTargetDrift, convergence: ConvergenceDecisionRequired, kind: deployment.FileSecret, want: overwriteSkipAbort}, - {name: "file conflict", path: decisionPath, action: ActionNeedsDecision, reason: ReasonConflict, convergence: ConvergenceDecisionRequired, kind: deployment.FileOrdinary, want: diffOverwriteSkipAbort}, - {name: "file unbaselined differ", path: decisionPath, action: ActionNeedsDecision, reason: ReasonUnbaselinedDiffer, convergence: ConvergenceDecisionRequired, kind: deployment.FileOrdinary, want: diffOverwriteSkipAbort}, + {name: "file conflict", path: decisionPath, action: ActionNeedsDecision, reason: ReasonConflict, convergence: ConvergenceDecisionRequired, kind: deployment.FileOrdinary, want: overwriteSkipAbort}, + {name: "file unbaselined differ", path: decisionPath, action: ActionNeedsDecision, reason: ReasonUnbaselinedDiffer, convergence: ConvergenceDecisionRequired, kind: deployment.FileOrdinary, want: overwriteSkipAbort}, {name: "file unexpected type", path: decisionPath, action: ActionNeedsDecision, reason: ReasonUnexpectedTargetType, convergence: ConvergenceDecisionRequired, kind: deployment.FileOrdinary, want: overwriteSkipAbort}, {name: "file converged rejected", path: decisionPath, action: ActionNoOp, reason: ReasonNoChange, convergence: ConvergenceConverged, kind: deployment.FileOrdinary, wantErr: true}, {name: "file pending rejected", path: decisionPath, action: ActionWriteSourceToTarget, reason: ReasonSourceChanged, convergence: ConvergencePending, kind: deployment.FileOrdinary, wantErr: true}, @@ -118,13 +117,10 @@ type invalidCase struct { // invalidDecisionCases enumerates validation: a spec is rejected unless its // choices are exactly the allowed set for its action, reason, and kind. var invalidDecisionCases = []invalidCase{ - {name: "diff on secret", spec: specOf(ReasonTargetDrift, diffOverwriteSkipAbort), kind: deployment.FileSecret, wantErr: true}, {name: "duplicate overwrite", spec: specOf(ReasonTargetDrift, []DecisionChoice{ChoiceOverwrite, ChoiceOverwrite, ChoiceSkip, ChoiceAbort}), kind: deployment.FileOrdinary, wantErr: true}, - {name: "missing abort", spec: specOf(ReasonTargetDrift, []DecisionChoice{ChoiceDiff, ChoiceOverwrite, ChoiceSkip}), kind: deployment.FileOrdinary, wantErr: true}, - {name: "diff on alias reason", spec: specOf(ReasonAliasWrong, diffOverwriteSkipAbort), kind: deployment.FileOrdinary, wantErr: true}, - {name: "diff on unexpected type", spec: specOf(ReasonUnexpectedTargetType, diffOverwriteSkipAbort), kind: deployment.FileOrdinary, wantErr: true}, + {name: "missing abort", spec: specOf(ReasonTargetDrift, []DecisionChoice{ChoiceOverwrite, ChoiceSkip}), kind: deployment.FileOrdinary, wantErr: true}, {name: "choices on automatic row", spec: specOf(ReasonNoChange, overwriteSkipAbort), kind: deployment.FileOrdinary, wantErr: true}, - {name: "valid ordinary drift", spec: specOf(ReasonTargetDrift, diffOverwriteSkipAbort), kind: deployment.FileOrdinary}, + {name: "valid ordinary drift", spec: specOf(ReasonTargetDrift, overwriteSkipAbort), kind: deployment.FileOrdinary}, {name: "valid secret drift", spec: specOf(ReasonTargetDrift, overwriteSkipAbort), kind: deployment.FileSecret}, {name: "valid alias wrong", spec: specOf(ReasonAliasWrong, overwriteSkipAbort), kind: deployment.FileSecret}, } @@ -149,7 +145,7 @@ func specOf(reason Reason, choices []DecisionChoice) DecisionSpec { // orderSpec builds one ordinary drift spec at path. func orderSpec(path string) DecisionSpec { - return mustDecisionSpec(DecisionSpecInput{TargetPath: path, Action: ActionNeedsDecision, Reason: ReasonTargetDrift, Choices: diffOverwriteSkipAbort}) + return mustDecisionSpec(DecisionSpecInput{TargetPath: path, Action: ActionNeedsDecision, Reason: ReasonTargetDrift, Choices: overwriteSkipAbort}) } // checkDecisionSpec produces one spec from its row and compares the result. diff --git a/internal/reconcile/types.go b/internal/reconcile/types.go index f03ed58..7387059 100644 --- a/internal/reconcile/types.go +++ b/internal/reconcile/types.go @@ -95,10 +95,9 @@ const ( ChoiceOverwrite DecisionChoice = iota ChoiceSkip ChoiceAbort - ChoiceDiff ) -func (c DecisionChoice) Valid() bool { return c >= ChoiceOverwrite && c <= ChoiceDiff } +func (c DecisionChoice) Valid() bool { return c >= ChoiceOverwrite && c <= ChoiceAbort } // DecisionSpec is one immutable, ordered resolution request: the target path, // the action and reason that produced it, and the choices the user may pick. diff --git a/internal/reconcile/types_test.go b/internal/reconcile/types_test.go index 7e2b425..54a3f85 100644 --- a/internal/reconcile/types_test.go +++ b/internal/reconcile/types_test.go @@ -72,7 +72,7 @@ func testSmallEnumValidity(t *testing.T) { t.Fatalf("known convergence %d must be valid", convergence) } } - choices := []DecisionChoice{ChoiceOverwrite, ChoiceSkip, ChoiceAbort, ChoiceDiff} + choices := []DecisionChoice{ChoiceOverwrite, ChoiceSkip, ChoiceAbort} for _, choice := range choices { if !choice.Valid() { t.Fatalf("known choice %d must be valid", choice) @@ -112,7 +112,7 @@ func testContentToken(t *testing.T) { } func testDecisionSpecCopy(t *testing.T) { - choices := []DecisionChoice{ChoiceOverwrite, ChoiceSkip, ChoiceAbort, ChoiceDiff} + choices := []DecisionChoice{ChoiceOverwrite, ChoiceSkip, ChoiceAbort} spec, err := NewDecisionSpec(DecisionSpecInput{ TargetPath: ".config/app/config", Action: ActionNeedsDecision, Reason: ReasonTargetDrift, Choices: choices, @@ -120,8 +120,8 @@ func testDecisionSpecCopy(t *testing.T) { if err != nil { t.Fatalf("NewDecisionSpec: %v", err) } - choices[0] = ChoiceDiff - if len(spec.AllChoices()) != 4 || spec.AllChoices()[0] != ChoiceOverwrite { + choices[0] = ChoiceAbort + if len(spec.AllChoices()) != 3 || spec.AllChoices()[0] != ChoiceOverwrite { t.Fatal("spec must copy its choice slice defensively") } returned := spec.AllChoices() -- 2.51.2