feat: delete DOProvider.Apply v1 wrapper; release v1.4.0 (Phase 3)

internal/deferred_test_helpers_test.go

@@ -0,0 +1,74 @@
+package internal
+
+// Test-only fixtures for the deferred-update flush path. Used by
+// iacserver_finalize_test.go (workflow#695 Phase 2.5 FinalizeApply
+// regression suite). Extracted from provider_deferred_test.go in the
+// Phase 3 cleanup that deleted the v1-wrapper DOProvider.Apply tests
+// (docs/plans/2026-05-17-phase2.5-cleanup-bundle.md Task 5) — the
+// fixtures themselves remain in-package so the v2-dispatch tests
+// continue to exercise the canonical DatabaseDriver mock surface
+// without coupling to the (now-stub) v1 Apply wrapper.
+
+import (
+	"context"
+
+	"github.com/digitalocean/godo"
+)
+
+// fakeAppForDeferred constructs a minimal *godo.App for deferred-test mocks.
+func fakeAppForDeferred(name, id string) *godo.App {
+	return &godo.App{ID: id, Spec: &godo.AppSpec{Name: name}}
+}
+
+// minimalDBMock satisfies drivers.DatabaseClient for the FinalizeApply
+// deferred-flush tests. Stores the last UpdateFirewallRules request so
+// the test can assert the flush occurred. The optional flushErr field —
+// when non-nil — is returned from UpdateFirewallRules so tests can
+// exercise the per-driver error-attribution path (workflow#695 Phase 2.5
+// FinalizeApply regression test).
+type minimalDBMock struct {
+	db              *godo.Database
+	lastFirewallReq *godo.DatabaseUpdateFirewallRulesRequest
+	flushErr        error
+}
+
+func (m *minimalDBMock) Create(_ context.Context, _ *godo.DatabaseCreateRequest) (*godo.Database, *godo.Response, error) {
+	return m.db, nil, nil
+}
+func (m *minimalDBMock) Get(_ context.Context, _ string) (*godo.Database, *godo.Response, error) {
+	return m.db, nil, nil
+}
+func (m *minimalDBMock) List(_ context.Context, _ *godo.ListOptions) ([]godo.Database, *godo.Response, error) {
+	if m.db == nil {
+		return nil, nil, nil
+	}
+	return []godo.Database{*m.db}, nil, nil
+}
+func (m *minimalDBMock) Resize(_ context.Context, _ string, _ *godo.DatabaseResizeRequest) (*godo.Response, error) {
+	return nil, nil
+}
+func (m *minimalDBMock) Delete(_ context.Context, _ string) (*godo.Response, error) {
+	return nil, nil
+}
+func (m *minimalDBMock) UpdateFirewallRules(_ context.Context, _ string, req *godo.DatabaseUpdateFirewallRulesRequest) (*godo.Response, error) {
+	m.lastFirewallReq = req
+	if m.flushErr != nil {
+		return nil, m.flushErr
+	}
+	return nil, nil
+}
+
+// sequencedAppsForDeferred returns empty on the first call, then the app list.
+// Simulates: DB create call (app absent) → flush call (app present).
+type sequencedAppsForDeferred struct {
+	apps      []*godo.App
+	callCount int
+}
+
+func (m *sequencedAppsForDeferred) List(_ context.Context, _ *godo.ListOptions) ([]*godo.App, *godo.Response, error) {
+	m.callCount++
+	if m.callCount == 1 {
+		return []*godo.App{}, nil, nil // first call: app doesn't exist yet
+	}
+	return m.apps, nil, nil
+}

internal/iacserver.go

@@ -183,13 +183,24 @@ func (s *doIaCServer) Capabilities(_ context.Context, _ *pb.CapabilitiesRequest)
 	}, nil
 }
 
+// deferredUpdater is the per-driver opt-in interface for resources that
+// accumulate updates that cannot be applied until all plan creates
+// complete. The canonical case is DatabaseDriver deferring type=app
+// trusted_sources entries that reference apps created later in the
+// same plan. doIaCServer.FinalizeApply iterates the driver registry
+// and calls FlushDeferredUpdates on every driver that opted in.
+// Per workflow#695 Phase 2.5 (relocated from provider.go in Phase 3).
+type deferredUpdater interface {
+	HasDeferredUpdates() bool
+	FlushDeferredUpdates(ctx context.Context) error
+}
+
 // FinalizeApply implements pb.IaCProviderFinalizerServer for the v2
-// dispatch path. Inlines the per-driver flush loop from
-// DOProvider.Apply (internal/provider.go's post-loop block iterating
-// p.drivers and calling deferredUpdater.FlushDeferredUpdates) since
-// DOProvider does not expose a public FlushDeferredUpdates method —
-// the flush iteration was inline in the v1 Apply wrapper. Per
-// workflow#695 Phase 2.5.
+// dispatch path. Iterates the driver registry and calls
+// FlushDeferredUpdates on every driver that satisfies deferredUpdater
+// (declared above). Originally inline in v1 DOProvider.Apply's post-loop
+// block; hoisted here for the v2 dispatch path. Per workflow#695
+// Phase 2.5.
 //
 // Per-driver error attribution is preserved by returning ActionError
 // entries on the response, mirroring the v1 wrapper shape that wfctl

internal/provider.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/GoCodeAlone/workflow-plugin-digitalocean/internal/drivers"
 	"github.com/GoCodeAlone/workflow-plugin-digitalocean/internal/steps"
-	"github.com/GoCodeAlone/workflow/iac/wfctlhelpers"
 	"github.com/GoCodeAlone/workflow/interfaces"
 	"github.com/GoCodeAlone/workflow/platform"
 	"github.com/digitalocean/godo"
@@ -34,6 +33,11 @@ func (t *tokenSource) Token() (*oauth2.Token, error) {
 // the legacy doPlugin / NewDOPlugin entrypoint was deleted (Task 9).
 var Version = "dev"
 
+// ErrApplyV1Removed is the sentinel returned by DOProvider.Apply when
+// called post-Phase-3 cleanup. Callers can errors.Is for diagnostic
+// classification.
+var ErrApplyV1Removed = errors.New("DOProvider.Apply: v1 dispatch removed since DO v1.4.0 (workflow#695 Phase 2.5 + Phase 3); upgrade wfctl to v0.55.0+ for v2 dispatch via wfctlhelpers.ApplyPlanWithHooks + IaCProviderFinalizer.FinalizeApply")
+
 // DOProvider implements interfaces.IaCProvider for DigitalOcean.
 type DOProvider struct {
 	client  *godo.Client
@@ -225,90 +229,17 @@ func (p *DOProvider) Plan(ctx context.Context, desired []interfaces.ResourceSpec
 	return &plan, err
 }
 
-// deferredUpdater is an optional interface for ResourceDrivers that accumulate
-// resource-level updates that cannot be applied until all plan creates complete.
-// The canonical case is DatabaseDriver deferring type=app trusted_sources entries
-// that reference apps created later in the same plan. Apply calls
-// FlushDeferredUpdates once after the main dispatch loop; errors are appended to
-// ApplyResult.Errors so the failure is visible to the operator.
-//
-// This is a DO-plugin-specific extension point not yet hoisted into
-// wfctlhelpers.ApplyPlan; the second-pass flush below preserves the regression
-// gate while the v2 dispatch handles the per-action loop.
-type deferredUpdater interface {
-	HasDeferredUpdates() bool
-	FlushDeferredUpdates(ctx context.Context) error
-}
-
-// Apply executes the plan via wfctlhelpers.ApplyPlan, then runs the
-// DO-specific deferred-update second pass.
-//
-// PR P-DO TP2: under iacProvider.computePlanVersion: v2 wfctl dispatches
-// directly through wfctlhelpers.ApplyPlan and does not call this method.
-// The implementation here remains for legacy v1 callers (wfctl < v0.21.0
-// or any in-process embedder of the gRPC plugin). v2 callers route the
-// deferred-flush through the FinalizeApply RPC (see iacserver.go); the
-// workflow engine invokes it via the ApplyPlanHooks.OnPlanComplete hook.
-// Per workflow#695 Phase 2.5.
-//
-// Per-action upsert recovery, JIT substitution, the Replace cascade, and
-// the input-drift postcondition all live in wfctlhelpers.ApplyPlan now —
-// drivers that opt into the upsert recovery path implement
-// interfaces.UpsertSupporter (DO drivers AppPlatform, VPC, Firewall,
-// Database all do; signature: SupportsUpsert() bool). The local
-// upsertSupporter interface previously declared here is no longer needed:
-// its SupportsUpsert() bool method is structurally identical to
-// interfaces.UpsertSupporter, so the existing driver implementations
-// satisfy the canonical interface without code change.
-//
-// wfctl:skip-iac-codemod
-//
-// The body intentionally wraps wfctlhelpers.ApplyPlan rather than
-// matching the codemod's canonical single-statement
-// `return wfctlhelpers.ApplyPlan(ctx, p, plan)` shape: the
-// post-helper deferred-update flush below is a DO-plugin-specific
-// regression gate (see provider_deferred_test.go and CHANGELOG entry
-// for staging-deploy-blockers Blocker 2) that wfctlhelpers does not
-// hoist. The skip marker tells the codemod's
-// AssertApplyDelegatesToHelper analyzer this deviation is intentional.
-// When wfctlhelpers grows a deferred-update lifecycle hook, the
-// wrapper can collapse and the marker can drop.
-func (p *DOProvider) Apply(ctx context.Context, plan *interfaces.IaCPlan) (*interfaces.ApplyResult, error) {
-	result, err := wfctlhelpers.ApplyPlan(ctx, p, plan)
-	if err != nil {
-		// ApplyPlan only returns a top-level error on context cancellation
-		// — per-action failures land on result.Errors. Surface the
-		// cancellation alongside whatever partial result is in hand so
-		// callers can still inspect any actions that completed.
-		return result, err
-	}
-
-	// Second pass: flush deferred updates accumulated by drivers during the
-	// main action loop. These arise when a resource's config (e.g. DB
-	// trusted_sources with type=app) references another resource provisioned
-	// later in the same plan. By this point all plan creates have completed
-	// and the referenced resources exist.
-	//
-	// Iterate the driver registry directly (not plan.Actions) so that orphaned
-	// deferred entries are flushed even when their resource type no longer
-	// appears in the current plan (e.g. after a transient flush failure on a
-	// prior Apply run). Each driver is checked at most once regardless of how
-	// many actions reference it.
-	for resourceType, d := range p.drivers {
-		du, ok := d.(deferredUpdater)
-		if !ok || !du.HasDeferredUpdates() {
-			continue
-		}
-		if flushErr := du.FlushDeferredUpdates(ctx); flushErr != nil {
-			result.Errors = append(result.Errors, interfaces.ActionError{
-				Resource: resourceType,
-				Action:   "deferred_update",
-				Error:    flushErr.Error(),
-			})
-		}
-	}
-
-	return result, nil
+// Apply returns ErrApplyV1Removed unconditionally. v1 dispatch was
+// removed in DO v1.4.0 (workflow#695 Phase 3 cleanup). wfctl bypasses
+// this method when ComputePlanVersion="v2" is declared in Capabilities;
+// deferred-flush behavior moved to doIaCServer.FinalizeApply via
+// IaCProviderFinalizer RPC. Reaching this method indicates a
+// misconfigured caller (in-process embedder using a pre-Phase-2.5
+// wfctl tag, OR gRPC consumer that opted out of v2 dispatch).
+// Stub preserves interfaces.IaCProvider contract per ADR 0024;
+// interface segregation deferred to separate refactor design.
+func (p *DOProvider) Apply(_ context.Context, _ *interfaces.IaCPlan) (*interfaces.ApplyResult, error) {
+	return nil, ErrApplyV1Removed
 }
 
 // EnumerateByTag implements the opt-in interfaces.Enumerator interface.

internal/provider_apply_stub_test.go

@@ -0,0 +1,22 @@
+package internal
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/GoCodeAlone/workflow/interfaces"
+)
+
+// TestDOProvider_Apply_ReturnsRemovedError pins the Phase 3 cleanup
+// invariant: DOProvider.Apply is dead code post-Phase-2.5 v2 cutover;
+// reaching it indicates a misconfigured caller. Method returns sentinel
+// ErrApplyV1Removed; callers can errors.Is for diagnostic classification.
+// Per workflow#695 Phase 3 cleanup + docs/plans/2026-05-17-phase2.5-cleanup-bundle.md.
+func TestDOProvider_Apply_ReturnsRemovedError(t *testing.T) {
+	p := &DOProvider{}
+	_, err := p.Apply(context.Background(), &interfaces.IaCPlan{})
+	if !errors.Is(err, ErrApplyV1Removed) {
+		t.Errorf("expected ErrApplyV1Removed; got: %v", err)
+	}
+}