From 488d85d8286c47692f06d1e8af6efb3f91772e6c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 10:21:02 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(iac):=20workflow#640=20Phase=205=20?= =?UTF-8?q?=E2=80=94=20remove=20legacy=20wfctlhelpers.ApplyPlan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md. Phase 1 (PR #691) + Phase 2 (PR #694) + Phase 2.5 (PR #697) + Phase 3 (per-plugin migration) all shipped. Production code has been on ApplyPlanWithHooks for weeks. This PR closes the loop by removing the deprecated symbol. Changes: iac/wfctlhelpers/apply.go: - Delete func ApplyPlan (the 2-line wrapper that delegated to applyPlanWithEnvProvider with nil hooks). - Keep applyPlanWithEnvProvider as the postcondition-test seam — retained for apply_postcondition_test.go's panicky-env injection. iac/wfctlhelpers/*_test.go: - 10 test files migrated from `ApplyPlan(ctx, p, plan)` to `ApplyPlanWithHooks(ctx, p, plan, ApplyPlanHooks{})`. Empty-hooks form is semantically identical to the pre-deletion ApplyPlan call. docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md: - Phase 5 marker flipped to SHIPPED. Verification: GOWORK=off go test ./... -count=1 (all green) Co-Authored-By: Claude Opus 4.7 (1M context) --- ...2026-05-16-v2-lifecycle-phase1-inventory.md | 2 +- iac/wfctlhelpers/apply.go | 18 ++++-------------- iac/wfctlhelpers/apply_create_test.go | 12 ++++++------ iac/wfctlhelpers/apply_jit_test.go | 10 +++++----- iac/wfctlhelpers/apply_postcondition_test.go | 10 +++++----- iac/wfctlhelpers/apply_replace_cascade_test.go | 4 ++-- iac/wfctlhelpers/apply_replace_test.go | 12 ++++++------ iac/wfctlhelpers/apply_test.go | 14 +++++++------- iac/wfctlhelpers/apply_update_delete_test.go | 12 ++++++------ 9 files changed, 42 insertions(+), 52 deletions(-) diff --git a/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md b/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md index 2cb982ee..8cede331 100644 --- a/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md +++ b/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md @@ -14,7 +14,7 @@ PR #639 landed the v2 action lifecycle hook path (`wfctlhelpers.ApplyPlanWithHoo - **Phase 2 — pending**: design + ship v2-hooks-over-gRPC contract (HARD-CUTOVER per ADR 0024) - **Phase 3 — pending**: migrate plugins to v2 contract (per-plugin manual + codemod for canonical-form plugins) - ~~Phase 4~~ — **SHIPPED in this PR**: migrated 3 conformance scenarios + 1 test file from `ApplyPlan` to `ApplyPlanWithHooks(..., ApplyPlanHooks{})`. Folded into Phase 1 because staticcheck SA1019 (from the new godoc marker) required it -- **Phase 5 — pending**: remove `wfctlhelpers.ApplyPlan` entirely (after Phase 2 + Phase 3 land) +- **Phase 5 — SHIPPED in this PR**: removed `wfctlhelpers.ApplyPlan` and migrated 10 internal test files to `ApplyPlanWithHooks(..., ApplyPlanHooks{})`. v1 apply path fully retired. ## Workflow-side caller inventory diff --git a/iac/wfctlhelpers/apply.go b/iac/wfctlhelpers/apply.go index 15f88e2f..3c7d9e39 100644 --- a/iac/wfctlhelpers/apply.go +++ b/iac/wfctlhelpers/apply.go @@ -92,18 +92,6 @@ import ( // T3.1 ships the dispatch skeleton; T3.1.5 added the postcondition above; // T3.2/T3.3/T3.4 fill the per-action sub-functions with their full bodies. // -// Deprecated: use [ApplyPlanWithHooks] instead. ApplyPlan is the empty-hooks -// equivalent of ApplyPlanWithHooks and is preserved only for backwards -// compatibility during the workflow#640 v2 action lifecycle migration. New -// callers MUST use ApplyPlanWithHooks so per-action state-persistence hooks -// can fire at each cloud-mutation boundary. See -// docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md and -// decisions/0040-v2-action-lifecycle-provider-compatibility.md for the -// migration contract; ApplyPlan will be removed in Phase 5. -func ApplyPlan(ctx context.Context, p interfaces.IaCProvider, plan *interfaces.IaCPlan) (*interfaces.ApplyResult, error) { - return applyPlanWithEnvProvider(ctx, p, plan, nil) -} - // ApplyPlanHooks are optional callbacks invoked immediately after a plan action // successfully mutates cloud-side state. Hooks let wfctl persist state at the // action boundary instead of waiting for the whole plan to finish. @@ -151,8 +139,10 @@ func ApplyPlanWithHooks(ctx context.Context, p interfaces.IaCProvider, plan *int // into the deferred drift postcondition (e.g., a panicky closure that // stresses the recover() guard). When applyTimeEnv is nil, the function // uses inputsnapshot.NewTolerantEnvProvider(plan.InputSnapshot) — the -// production behavior. The seam stays unexported because the only -// sanctioned external entry point is ApplyPlan. +// production behavior. +// +// Production callers use ApplyPlanWithHooks. This seam is retained +// purely so the postcondition test can inject a panicky env-provider. func applyPlanWithEnvProvider( ctx context.Context, p interfaces.IaCProvider, diff --git a/iac/wfctlhelpers/apply_create_test.go b/iac/wfctlhelpers/apply_create_test.go index bb03f14f..51376941 100644 --- a/iac/wfctlhelpers/apply_create_test.go +++ b/iac/wfctlhelpers/apply_create_test.go @@ -85,7 +85,7 @@ func TestApplyPlan_Create_UpsertOnAlreadyExists(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "create", Resource: spec("a", "infra.vpc")}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -110,7 +110,7 @@ func TestApplyPlan_Create_AlreadyExists_NoUpsertSupport(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "create", Resource: spec("a", "infra.vpc")}, }} - result, _ := ApplyPlan(context.Background(), fp, plan) + result, _ := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if len(result.Errors) != 1 { t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) } @@ -159,7 +159,7 @@ func TestApplyPlan_Create_AlreadyExists_DriverDoesNotImplementUpsertSupporter(t plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "create", Resource: spec("a", "infra.vpc")}, }} - result, _ := ApplyPlan(context.Background(), fp, plan) + result, _ := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if len(result.Errors) != 1 { t.Fatalf("expected 1 per-action error; got %d (%v)", len(result.Errors), result.Errors) } @@ -195,7 +195,7 @@ func TestApplyPlan_Create_UpsertReadFailureWraps(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "create", Resource: spec("a", "infra.vpc")}, }} - result, _ := ApplyPlan(context.Background(), fp, plan) + result, _ := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if len(result.Errors) != 1 { t.Fatalf("expected 1 error; got %d (%v)", len(result.Errors), result.Errors) } @@ -226,7 +226,7 @@ func TestApplyPlan_Create_UpsertEmptyProviderIDFails(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "create", Resource: spec("vpc-1", "infra.vpc")}, }} - result, _ := ApplyPlan(context.Background(), fp, plan) + result, _ := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if len(result.Errors) != 1 { t.Fatalf("expected 1 error; got %d (%v)", len(result.Errors), result.Errors) } @@ -257,7 +257,7 @@ func TestApplyPlan_Create_UpsertAppendsResources(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "create", Resource: spec("vpc-1", "infra.vpc")}, }} - result, _ := ApplyPlan(context.Background(), fp, plan) + result, _ := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if len(result.Resources) != 1 { t.Fatalf("expected 1 resource appended; got %d (%+v)", len(result.Resources), result.Resources) } diff --git a/iac/wfctlhelpers/apply_jit_test.go b/iac/wfctlhelpers/apply_jit_test.go index 8938e0aa..e3900bcf 100644 --- a/iac/wfctlhelpers/apply_jit_test.go +++ b/iac/wfctlhelpers/apply_jit_test.go @@ -96,7 +96,7 @@ func TestApplyPlan_JIT_TwoCreate_BSpecResolvesAID(t *testing.T) { })}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -145,7 +145,7 @@ func TestApplyPlan_JIT_PreSyncedFromActionCurrentState(t *testing.T) { Outputs: map[string]any{"cidr": "10.0.0.0/16", "region": "nyc3"}, } - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -181,7 +181,7 @@ func TestApplyPlan_JIT_UnresolvedRef_RecordsActionErrorAndSkipsDispatch(t *testi "vpc_uuid": "${ghost.id}", // ghost has no state, no replace-id })}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -216,7 +216,7 @@ func TestApplyPlan_JIT_NoRefsInConfig_PassesThroughUnchanged(t *testing.T) { "region": "nyc3", })}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -244,7 +244,7 @@ func TestApplyPlan_JIT_LoopContinuesAfterPerActionJITError(t *testing.T) { "cidr": "10.0.0.0/16", })}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } diff --git a/iac/wfctlhelpers/apply_postcondition_test.go b/iac/wfctlhelpers/apply_postcondition_test.go index e45e3dad..e5fa14f0 100644 --- a/iac/wfctlhelpers/apply_postcondition_test.go +++ b/iac/wfctlhelpers/apply_postcondition_test.go @@ -37,7 +37,7 @@ func TestApplyPlan_InitialInputSnapshot_CapturedAtEntry(t *testing.T) { }, } fp := newFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -96,7 +96,7 @@ func TestApply_Postcondition_FingerprintAfterEnvUnset_NoFalsePositive(t *testing fakeProvider: newFakeProvider(), varToUnset: varName, } - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -149,7 +149,7 @@ func TestApplyPlan_PlanStaleDiagnostic_NamesChangedKeys(t *testing.T) { InputSnapshot: map[string]string{varName: planFP}, } fp := newFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -175,7 +175,7 @@ func TestApplyPlan_PlanStaleDiagnostic_NamesChangedKeys(t *testing.T) { func TestApplyPlan_NoDriftWhenInputSnapshotEmpty(t *testing.T) { plan := &interfaces.IaCPlan{} fp := newFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -205,7 +205,7 @@ func TestApplyPlan_PostconditionRunsEvenIfDispatchErrored(t *testing.T) { }, } fp := &erroringFakeProvider{fakeProvider: newFakeProvider()} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } diff --git a/iac/wfctlhelpers/apply_replace_cascade_test.go b/iac/wfctlhelpers/apply_replace_cascade_test.go index 9f8a639b..cd6614fe 100644 --- a/iac/wfctlhelpers/apply_replace_cascade_test.go +++ b/iac/wfctlhelpers/apply_replace_cascade_test.go @@ -49,7 +49,7 @@ func TestApplyPlan_ReplaceCascade_DependentCreateGetsNewParentID(t *testing.T) { }), }, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -107,7 +107,7 @@ func TestApplyPlan_ReplaceCascade_DependentReplaceGetsNewParentID(t *testing.T) Current: &interfaces.ResourceState{Name: "dependent", ProviderID: "droplet-old-uuid"}, }, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } diff --git a/iac/wfctlhelpers/apply_replace_test.go b/iac/wfctlhelpers/apply_replace_test.go index aaf12d91..dfe0f2c9 100644 --- a/iac/wfctlhelpers/apply_replace_test.go +++ b/iac/wfctlhelpers/apply_replace_test.go @@ -81,7 +81,7 @@ func TestApplyPlan_Replace_DeletesThenCreates_PropagatesNewID(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -111,7 +111,7 @@ func TestApplyPlan_Replace_PopulatesReplaceIDMap(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -135,7 +135,7 @@ func TestApplyPlan_Replace_MultipleActionsAllPopulate(t *testing.T) { fakeProvider: newFakeProvider(), newIDs: map[string]string{"vpc": "new-vpc-id", "db": "new-db-id"}, } - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -194,7 +194,7 @@ func TestApplyPlan_Replace_DeleteFailsDoesNotCreate(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -232,7 +232,7 @@ func TestApplyPlan_Replace_CtxCancelAfterDelete_SkipsCreate(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, }} - result, err := ApplyPlan(ctx, fp, plan) + result, err := ApplyPlanWithHooks(ctx, fp, plan, ApplyPlanHooks{}) if err != nil { // ApplyPlan's loop check catches the cancellation at the next // iteration, but the per-action ctx check inside doReplace @@ -297,7 +297,7 @@ func TestApplyPlan_Replace_CreateFailsLeavesMapEmpty(t *testing.T) { plan := &interfaces.IaCPlan{Actions: []interfaces.PlanAction{ {Action: "replace", Resource: spec("vpc", "infra.vpc"), Current: stateWithID("vpc", "old-uuid")}, }} - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } diff --git a/iac/wfctlhelpers/apply_test.go b/iac/wfctlhelpers/apply_test.go index 5432740c..9988c7cf 100644 --- a/iac/wfctlhelpers/apply_test.go +++ b/iac/wfctlhelpers/apply_test.go @@ -132,7 +132,7 @@ func TestApplyPlan_HandlesAllFourActions(t *testing.T) { }, } fp := newFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -167,7 +167,7 @@ func TestApplyPlan_ReplaceDispatchesViaDeleteThenCreate(t *testing.T) { }, } fp := newFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -197,7 +197,7 @@ func TestApplyPlan_UnknownActionRecordsError(t *testing.T) { }, } fp := newFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatalf("ApplyPlan should not return a top-level error for a per-action issue; got %v", err) } @@ -215,7 +215,7 @@ func TestApplyPlan_UnknownActionRecordsError(t *testing.T) { func TestApplyPlan_PreservesPlanID(t *testing.T) { plan := &interfaces.IaCPlan{ID: "plan-12345"} fp := newFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -240,7 +240,7 @@ func TestApplyPlan_ResolveDriverErrorRecordsActionError(t *testing.T) { } fp := newFakeProvider() fp.driverErr = errResolveDriver - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatalf("top-level error not expected for per-action driver-resolve failure; got %v", err) } @@ -283,7 +283,7 @@ func TestApplyPlan_LoopContinuesAfterPerActionFailure(t *testing.T) { {Action: "create", Resource: spec("good", "infra.vpc")}, }, } - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -329,7 +329,7 @@ func TestApplyPlan_CtxCancellationStopsLoop(t *testing.T) { }, } fp := newFakeProvider() - _, err := ApplyPlan(ctx, fp, plan) + _, err := ApplyPlanWithHooks(ctx, fp, plan, ApplyPlanHooks{}) if !errors.Is(err, context.Canceled) { t.Fatalf("expected context.Canceled top-level error; got %v", err) } diff --git a/iac/wfctlhelpers/apply_update_delete_test.go b/iac/wfctlhelpers/apply_update_delete_test.go index b9c38b50..4a379175 100644 --- a/iac/wfctlhelpers/apply_update_delete_test.go +++ b/iac/wfctlhelpers/apply_update_delete_test.go @@ -74,7 +74,7 @@ func TestApplyPlan_Update_PassesProviderID(t *testing.T) { }, } fp := newCaptureFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -110,7 +110,7 @@ func TestApplyPlan_Update_NilCurrentIsHandledDefensively(t *testing.T) { }, } fp := newCaptureFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -145,7 +145,7 @@ func TestApplyPlan_Delete_NilCurrentIsHandledDefensively(t *testing.T) { }, } fp := newCaptureFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -183,7 +183,7 @@ func TestApplyPlan_Delete_InvokesDriverDelete(t *testing.T) { }, } fp := newCaptureFakeProvider() - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -212,7 +212,7 @@ func TestApplyPlan_Delete_DriverErrorRecorded(t *testing.T) { } fp := newCaptureFakeProvider() fp.driver.deleteErr = deleteErr - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } @@ -243,7 +243,7 @@ func TestApplyPlan_Update_DriverErrorRecorded(t *testing.T) { } fp := newCaptureFakeProvider() fp.driver.updateErr = updateErr - result, err := ApplyPlan(context.Background(), fp, plan) + result, err := ApplyPlanWithHooks(context.Background(), fp, plan, ApplyPlanHooks{}) if err != nil { t.Fatal(err) } From 5413214bd2be09a23b6ffb2fa0e557cf502d3ce0 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 11:50:22 -0400 Subject: [PATCH 2/3] docs(iac): close v2 lifecycle migration notes --- ...026-05-16-v2-lifecycle-phase1-inventory.md | 116 ++++++------------ iac/wfctlhelpers/apply.go | 88 ++++++------- iac/wfctlhelpers/apply_replace_test.go | 4 +- plugin/external/proto/iac.pb.go | 10 +- plugin/external/proto/iac.proto | 10 +- plugin/sdk/manifest.go | 23 ++-- plugin/sdk/manifest_schema.json | 2 +- 7 files changed, 100 insertions(+), 153 deletions(-) diff --git a/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md b/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md index 8cede331..26691b85 100644 --- a/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md +++ b/docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md @@ -1,96 +1,56 @@ -# V2 Action Lifecycle — Phase 1 Inventory +# V2 Action Lifecycle — Migration Inventory And Final State -**Status:** Phase 1 complete 2026-05-16 +**Status:** Closed 2026-05-20 after workflow#699 and workflow#743. **Tracking issue:** GoCodeAlone/workflow#640 **Phase 1 design:** `docs/plans/2026-05-16-v2-lifecycle-phase1-design.md` **ADR:** `decisions/0040-v2-action-lifecycle-provider-compatibility.md` ## Background -PR #639 landed the v2 action lifecycle hook path (`wfctlhelpers.ApplyPlanWithHooks`) — wfctl can persist state at each successful cloud-mutation boundary instead of waiting for whole-plan completion. The pre-existing `wfctlhelpers.ApplyPlan` is preserved for backwards compatibility but is now legacy debt. +PR #639 introduced `wfctlhelpers.ApplyPlanWithHooks`, allowing wfctl to persist +state at each successful cloud-mutation boundary instead of waiting for +whole-plan completion. The migration is now complete: `wfctlhelpers.ApplyPlan` +has been removed, the v1 `provider.Apply` dispatch path was removed in +workflow#699, and `ApplyPlanWithHooks` is the only wfctl-side plan-execution +helper. -#640 tracks the multi-phase migration: -- **Phase 1 (this document) — SHIPPED**: inventory + provider-compatibility ADR + Deprecated marker -- **Phase 2 — pending**: design + ship v2-hooks-over-gRPC contract (HARD-CUTOVER per ADR 0024) -- **Phase 3 — pending**: migrate plugins to v2 contract (per-plugin manual + codemod for canonical-form plugins) -- ~~Phase 4~~ — **SHIPPED in this PR**: migrated 3 conformance scenarios + 1 test file from `ApplyPlan` to `ApplyPlanWithHooks(..., ApplyPlanHooks{})`. Folded into Phase 1 because staticcheck SA1019 (from the new godoc marker) required it -- **Phase 5 — SHIPPED in this PR**: removed `wfctlhelpers.ApplyPlan` and migrated 10 internal test files to `ApplyPlanWithHooks(..., ApplyPlanHooks{})`. v1 apply path fully retired. +## Shipped Phases -## Workflow-side caller inventory +| Phase | Final state | +| --- | --- | +| Phase 1 | Inventory, provider-compatibility ADR, and deprecation marker shipped 2026-05-16. | +| Phase 2 | Typed lifecycle hard-cutover shipped in workflow#699; loaders reject non-v2 providers. | +| Phase 3 | Plugin migrations shipped as part of the v2 lifecycle cutover. | +| Phase 4 | Conformance and in-tree test callers migrated to `ApplyPlanWithHooks(..., ApplyPlanHooks{})`. | +| Phase 5 | workflow#743 removed the legacy `wfctlhelpers.ApplyPlan` wrapper and cleaned remaining helper callers. | -### v1 `wfctlhelpers.ApplyPlan` callers (production) +## Final Caller Inventory -| File:Line | Classification | Notes | -|-----------|----------------|-------| -| `iac/conformance/scenario_upsert_on_already_exists.go:88` | MIGRATED in this PR (Phase 4 folded) | Renamed to `ApplyPlanWithHooks(..., ApplyPlanHooks{})`. Empty-hooks form has zero semantic difference. | -| `iac/conformance/scenario_delete_action.go:74` | MIGRATED in this PR | Same | -| `iac/conformance/scenario_replace_cascade_preserves_dependents.go:92` | MIGRATED in this PR | Same | -| `cmd/wfctl/infra_apply_in_process_test.go:77` | MIGRATED in this PR (was missing from initial inventory; surfaced by staticcheck SA1019) | Same | +| Surface | Final state | +| --- | --- | +| `wfctlhelpers.ApplyPlan` | Removed. No production or test callers remain. | +| `wfctlhelpers.ApplyPlanWithHooks` | Sole exported wfctl-side helper for v2 IaC plan execution. | +| `cmd/wfctl` provider dispatch | Uses the strict typed v2 lifecycle path; legacy `provider.Apply` dispatch was removed in workflow#699. | +| `cmd/iac-codemod` | Removed in workflow#699 because plugin-side `Apply` implementations no longer exist in the runtime contract. | +| Plugin capability gate | `CapabilitiesResponse.compute_plan_version` must be `"v2"`; empty, `"v1"`, and unrecognized values are rejected at load time. | -### ~~iac-codemod tool references (NOT runtime callers)~~ — superseded by workflow#699 +## Compatibility Outcome -> **Update 2026-05-17 (workflow#699):** `cmd/iac-codemod/` was deleted in PR 1 -> of the IaCProvider.Apply hard-removal cascade. The codemod's reason-to-exist -> (migrate v1 `Apply` impls to v2 `wfctlhelpers.ApplyPlan` delegation) -> evaporated when `IaCProvider.Apply` itself was removed from the interface + -> proto. The original references below are kept for historical context but -> are no longer actionable. +Per `decisions/0040-v2-action-lifecycle-provider-compatibility.md`, providers +must satisfy the v2 lifecycle invariants at the typed IaCProvider RPC boundary: -| File:Line | Notes | -|-----------|-------| -| `cmd/iac-codemod/refactor_apply.go:29` (`applyCanonicalCallExpr` constant, `//nolint:unused`) | ~~Phase 3 lockstep update bumps this constant.~~ DELETED per workflow#699. | -| `cmd/iac-codemod/refactor_apply.go:1208` (doc comment) | ~~Same Phase 3 lockstep update.~~ DELETED per workflow#699. | -| `cmd/iac-codemod/lint.go:54` (comment) + `lint.go:641` (matcher consumer) | ~~Same.~~ DELETED per workflow#699. | - -### v1 `provider.Apply(ctx, &plan)` direct callers (workflow side, NOT through wfctlhelpers) - -| File:Line | Classification | Notes | -|-----------|----------------|-------| -| `cmd/wfctl/infra_apply.go:486` | MIGRATE-NEEDED (Phase 2 — v1 dispatch path removal) | `else` branch sister to L473 `applyV2ApplyPlanWithHooksFn`. Selected by `DispatchVersionFor(provider) == DispatchVersionV2` predicate. v2 path uses `provider.ResourceDriver(action.Resource.Type)` per action — NOT `provider.Apply`. Phase 2 removes this v1 path entirely after all 4 plugins ship Phase 2-conformant Apply RPC + manifest declaration. | -| `cmd/wfctl/infra_apply.go:1615` | MIGRATE-NEEDED (Phase 2) | Same — second occurrence (likely refresh path). | -| `cmd/wfctl/iac_typed_adapter.go:350` | NOT IN V2 HOT PATH (Phase 2 wire-format work) | `typedIaCAdapter.Apply` is called ONLY on the v1 dispatch path. Per-action wire format change happens in `applyResultFromPB` (the typed adapter's response decoder) when Phase 2 extends `ApplyResponse` proto with per-action `Actions []ActionResult` field. Adapter dispatch shape doesn't need to change; response decoder does. | - -## Plugin-side IaCProvider.Apply implementation inventory (verified 2026-05-16 via `gh api`) - -| Plugin | File:Line | Pattern | Phase 3 path | -|--------|-----------|---------|--------------| -| workflow-plugin-aws | `provider/provider.go:237` `AWSProvider.Apply` | **NON-CANONICAL** — own loop with `p.mu.RLock` + init check + custom dispatch | **MIGRATE-NEEDED (Phase 3 MANUAL)** — codemod cannot rewrite | -| workflow-plugin-gcp | `provider/provider.go:226` `GCPProvider.Apply` | **NON-CANONICAL** — own `for _, action := range plan.Actions` with `p.ResourceDriver(action.Resource.Type)` per-action | **MIGRATE-NEEDED (Phase 3 MANUAL)** — codemod cannot rewrite | -| workflow-plugin-azure | `internal/provider.go:138` `AzureProvider.Apply` | **NON-CANONICAL** — own loop with `p.mu.RLock` | **MIGRATE-NEEDED (Phase 3 MANUAL)** — codemod cannot rewrite | -| workflow-plugin-digitalocean | `internal/provider.go:274-275` `DOProvider.Apply` | **CANONICAL** — `result, err := wfctlhelpers.ApplyPlan(ctx, p, plan)` delegate (with custom post-flush wrapper) | LEAVE-AS-IS for Phase 1; Phase 3 codemod CAN rewrite (after AST functions updated in lockstep with constant) | - -## Major architectural finding - -**3 of 4 IaC plugins do NOT use the iac-codemod canonical pattern (`return wfctlhelpers.ApplyPlan(ctx, p, plan)`).** The canonical-form constant in `cmd/iac-codemod/refactor_apply.go:29` is aspirational, not reality. - -Phase 2 + Phase 3 implications: -1. **Phase 3 is NOT a single codemod-fix-it run.** 3 plugins need MANUAL migration; 1 plugin (DO) can be codemod-rewritten. -2. **Phase 2 v2 hooks contract design** must accommodate two plugin implementation paths: - - **(a) Canonical delegate**: `provider.Apply` → `wfctlhelpers.ApplyPlan(ctx, p, plan)` → `applyPlanWithEnvProviderAndHooks(ctx, p, plan, nil, hooks)` — wfctl-side hooks fire automatically at each `dispatchAction` boundary - - **(b) Custom loop**: `provider.Apply` runs its own `for _, action := range plan.Actions` loop, calling `p.ResourceDriver(action.Resource.Type)` per-action. The plugin must EMIT per-action outcome via the Phase 2 extended `ApplyResponse` proto so wfctl-side reconstructs the hook events -3. **Phase 2 contract MUST be a hard-cutover per ADR 0024.** No graceful proto fallback — workflow + 4 plugin repos ship the new ApplyResponse shape simultaneously, same coordination pattern as the strict-contracts cutover. - -## Provider compatibility expectations (Phase 2 contract preview) - -Per `decisions/0040-v2-action-lifecycle-provider-compatibility.md`, plugins MUST satisfy these 5 invariants at the IaCProviderRequiredServer.Apply RPC boundary: - -1. **Per-action success evidence** — ApplyResponse MUST include per-action outcome (success/error per `PlanAction`) -2. **Failed-delete preservation** — Apply MUST flag failed-delete actions distinctly so wfctl `OnResourceDeleted` does NOT fire -3. **Compensation evidence** — Apply MUST include compensation outcome when create/replace persistence/routing fails -4. **Update-failure non-deletion** — Plugins MUST NOT pre-emptively delete on update failure (engine-side enforced; plugin must not override) -5. **ResourceReplacer advertisement** — Plugin manifest MUST advertise ResourceReplacer usage so wfctl pre-mutation gates correctly - -## Out of scope for Phase 1 - -- Phase 2 gRPC contract design + implementation — separate design pass -- Phase 3 plugin migration — separate per-plugin design + execution passes -- Phase 4 conformance scenario migration — separate trivial PR -- Phase 5 ApplyPlan removal — gates on Phase 4 completion + plugin canonical-form propagation +1. Per-action success evidence is present for hook replay. +2. Failed deletes do not trigger `OnResourceDeleted`. +3. Compensation evidence is available when persistence/routing fails. +4. Update failures are not treated as delete success. +5. ResourceReplacer usage is advertised so pre-mutation gates can run. ## References -- ADR 0024 (`decisions/0024-iac-typed-force-cutover.md`) — strict-contracts cutover precedent (no compat shim) -- ADR 0040 (`decisions/0040-v2-action-lifecycle-provider-compatibility.md`) — provider-side compatibility contract -- PR #639 — v2 hooks engine landing -- `iac/wfctlhelpers/apply.go` — engine source; package-level doc comment now embeds the v1/v2 contract pointer; `ApplyPlan` symbol carries `// Deprecated:` marker per Phase 1 +- ADR 0024 (`decisions/0024-iac-typed-force-cutover.md`) — strict-contracts cutover precedent. +- ADR 0040 (`decisions/0040-v2-action-lifecycle-provider-compatibility.md`) — provider-side compatibility contract. +- PR #639 — v2 hooks engine landing. +- PR #699 — strict v2 lifecycle and provider.Apply removal. +- PR #743 — `wfctlhelpers.ApplyPlan` removal. +- `iac/wfctlhelpers/apply.go` — current `ApplyPlanWithHooks` implementation. - Prior product design: `docs/plans/2026-04-25-wfctl-lifecycle-product-design.md` diff --git a/iac/wfctlhelpers/apply.go b/iac/wfctlhelpers/apply.go index 3c7d9e39..3252734a 100644 --- a/iac/wfctlhelpers/apply.go +++ b/iac/wfctlhelpers/apply.go @@ -1,37 +1,30 @@ // Package wfctlhelpers hosts the wfctl-side dispatch helper for v2 IaC -// plugins. wfctl calls [ApplyPlanWithHooks] (or the legacy [ApplyPlan]) when -// a plugin manifest declares iacProvider.computePlanVersion: v2 (see -// plugin/sdk.IaCProvider). The helper iterates plan.Actions, fetches the -// matching ResourceDriver from the provider, and dispatches each action to -// a per-action sub-function (doCreate, doUpdate, doReplace, doDelete). +// plugins. wfctl calls [ApplyPlanWithHooks] when a plugin's typed capability +// response declares compute_plan_version: "v2". The helper iterates +// plan.Actions, fetches the matching ResourceDriver from the provider, and +// dispatches each action to a per-action sub-function (doCreate, doUpdate, +// doReplace, doDelete). // // # Action lifecycle versions (workflow#640 migration) // -// Two callers can drive plan execution: -// -// - [ApplyPlan] (legacy, marked Deprecated) — empty-hooks equivalent -// of [ApplyPlanWithHooks]. State persistence happens at whole-plan -// completion only. -// -// - [ApplyPlanWithHooks] (v2, recommended) — caller-supplied per-action -// OnResourceApplied / OnResourceDeleted hooks fire at each successful -// cloud-mutation boundary. Required for #640's invariants. +// [ApplyPlanWithHooks] is the only exported plan-execution helper. Its +// caller-supplied per-action OnResourceApplied / OnResourceDeleted hooks fire +// at each successful cloud-mutation boundary. Required for #640's invariants. // // See docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md and // decisions/0040-v2-action-lifecycle-provider-compatibility.md for the -// migration contract; ApplyPlan will be removed in Phase 5. +// final migration closeout. // // Lifecycle inside W-3a: // -// - T3.1 (this file's ApplyPlan + dispatch + skeleton sub-functions) -// - T3.1.5 — wraps ApplyPlan with the input-drift postcondition +// - T3.1 (this file's dispatch helper + skeleton sub-functions) +// - T3.1.5 — wraps dispatch with the input-drift postcondition // - T3.2 — fills doCreate with UpsertSupporter recovery // - T3.3 — fills doUpdate + doDelete (the latent doDelete bug fix) // - T3.4 — fills doReplace and populates ApplyResult.ReplaceIDMap // -// Until W-3b lands the cmd/wfctl dispatch wiring, [ApplyPlan] has no -// in-tree caller — the helper ships in W-3a as foundation only and is -// exercised solely by this package's tests. +// workflow#743 removed the former ApplyPlan wrapper after all runtime paths +// moved to ApplyPlanWithHooks. // // # Per-action error-prefix policy // @@ -68,15 +61,15 @@ import ( "github.com/GoCodeAlone/workflow/interfaces" ) -// ApplyPlan dispatches each plan action to the matching ResourceDriver on -// the provider. Per-action errors are recorded on result.Errors and do NOT -// abort the loop — apply best-effort across actions, surface every failure -// for the operator to triage. Context cancellation between actions IS -// respected: when ctx is canceled or its deadline expires, the loop stops -// at the next iteration boundary and returns ctx.Err() as the top-level -// error so a long apply terminates promptly on Ctrl-C / SIGTERM. +// ApplyPlanWithHooks dispatches each plan action to the matching ResourceDriver +// on the provider. Per-action errors are recorded on result.Errors and do NOT +// abort the loop — apply best-effort across actions, surface every failure for +// the operator to triage. Context cancellation between actions IS respected: +// when ctx is canceled or its deadline expires, the loop stops at the next +// iteration boundary and returns ctx.Err() as the top-level error so a long +// apply terminates promptly on Ctrl-C / SIGTERM. // -// At entry ApplyPlan captures result.InitialInputSnapshot by fingerprinting +// At entry the helper captures result.InitialInputSnapshot by fingerprinting // every name listed in plan.InputSnapshot through the OS env. After the // dispatch loop completes — successfully or not — a deferred postcondition // computes result.InputDriftReport against an apply-time snapshot taken @@ -86,11 +79,11 @@ import ( // on panic, InputDriftReport is reset to nil and a warning is logged. // // The function is concurrency-safe with respect to its inputs: result is -// owned by ApplyPlan for the duration of the call and is not shared with -// the provider or driver implementations. +// owned by the helper for the duration of the call and is not shared with the +// provider or driver implementations. // -// T3.1 ships the dispatch skeleton; T3.1.5 added the postcondition above; -// T3.2/T3.3/T3.4 fill the per-action sub-functions with their full bodies. +// T3.1 shipped the dispatch skeleton; T3.1.5 added the postcondition above; +// T3.2/T3.3/T3.4 filled the per-action sub-functions with their full bodies. // // ApplyPlanHooks are optional callbacks invoked immediately after a plan action // successfully mutates cloud-side state. Hooks let wfctl persist state at the @@ -110,10 +103,9 @@ type ApplyPlanHooks struct { // - The per-action loop's `if fatalErr != nil { return ... }` // early-return — outer err != nil. v1 semantic preservation per // cycle-1 plan-review C-3: DOProvider.Apply skips deferred-flush - // when wfctlhelpers.ApplyPlan returns a top-level err (the - // `if err != nil { return ... }` guard in DOProvider.Apply - // immediately after the ApplyPlan call in - // workflow-plugin-digitalocean internal/provider.go). + // when ApplyPlanWithHooks returns a top-level err (the + // `if err != nil { return ... }` guard in the caller immediately + // after the helper call). // - The post-loop length-invariant check that compares // len(result.Actions) against len(plan.Actions) — outer err != nil. // @@ -128,8 +120,8 @@ type ApplyPlanHooks struct { OnPlanComplete func(context.Context) error } -// ApplyPlanWithHooks is ApplyPlan plus action-boundary hooks for callers that -// need durable side effects as each cloud mutation succeeds. +// ApplyPlanWithHooks dispatches plan actions plus action-boundary hooks for +// callers that need durable side effects as each cloud mutation succeeds. func ApplyPlanWithHooks(ctx context.Context, p interfaces.IaCProvider, plan *interfaces.IaCPlan, hooks ApplyPlanHooks) (*interfaces.ApplyResult, error) { return applyPlanWithEnvProviderAndHooks(ctx, p, plan, nil, hooks) } @@ -175,10 +167,9 @@ func applyPlanWithEnvProviderAndHooks( // v1 semantic preservation: outer-error exits (the per-action // loop's `if fatalErr != nil { return ... }` early-return and // the post-loop length-invariant check) skip finalize, - // matching DOProvider.Apply's "return without flushing on - // top-level err" behavior (the `if err != nil { return ... }` - // guard immediately after the wrapped ApplyPlan call in - // workflow-plugin-digitalocean internal/provider.go). + // matching the legacy "return without flushing on top-level + // err" behavior (the `if err != nil { return ... }` guard + // immediately after the helper call). return } // Symmetry with the drift defer below: OnPlanComplete is a @@ -629,9 +620,9 @@ func dispatchAction(ctx context.Context, d interfaces.ResourceDriver, action int // - Read-after-conflict failures wrap both the original Create error // and the Read error via errors.Join, so callers in this package // can match either via errors.Is. -// - The doCreate return value preserves the wrap chain. ApplyPlan's +// - The doCreate return value preserves the wrap chain. The helper's // dispatch loop, however, flattens errors to a string in -// result.Errors[].Error (see [ApplyPlan]) — external callers +// result.Errors[].Error — external callers // reading [interfaces.ApplyResult].Errors lose errors.Is matching // and must inspect the canonical "upsert: read after conflict:" // prefix instead. This boundary is deliberate: ActionError carries @@ -665,7 +656,7 @@ func doCreate(ctx context.Context, d interfaces.ResourceDriver, action interface // ProviderID (when action.Current is non-nil), appending the driver's // returned ResourceOutput to result.Resources on success. Driver errors // pass through unchanged so the caller's per-action error wrapper -// (ApplyPlan's loop body) records them with the canonical action + +// (the helper loop body) records them with the canonical action + // resource fields. // // Defensive contract: doUpdate does NOT synthesize a precondition error @@ -693,7 +684,7 @@ func doUpdate(ctx context.Context, d interfaces.ResourceDriver, action interface // Decomposes a Replace action into Delete-then-Create on the driver and // propagates the new ProviderID through // result.ReplaceIDMap[action.Resource.Name] so the JIT substitution wired -// into ApplyPlan's loop (T5.2) can patch dependent resources whose +// into the helper loop (T5.2) can patch dependent resources whose // configs reference the replaced resource by name. // // # Cascade contract (T5.3) @@ -879,9 +870,8 @@ func hasReplaceErrorPrefix(err error) bool { // ProviderID. This closes the latent gap documented in the design // (DOProvider.Apply has no "case delete" arm today, so wfctl's // state-prune action silently skipped cloud-resource deletion through -// the v1 dispatch path); under v2 dispatch wfctlhelpers.ApplyPlan -// always invokes the driver's Delete, ensuring state-prune is paired -// with a real cloud-side mutation. +// the former v1 dispatch path); v2 dispatch always invokes the driver's +// Delete, ensuring state-prune is paired with a real cloud-side mutation. // // Driver errors pass through unchanged for the caller's per-action // error wrapping. doDelete does not append to result.Resources — a diff --git a/iac/wfctlhelpers/apply_replace_test.go b/iac/wfctlhelpers/apply_replace_test.go index dfe0f2c9..2d42583f 100644 --- a/iac/wfctlhelpers/apply_replace_test.go +++ b/iac/wfctlhelpers/apply_replace_test.go @@ -234,11 +234,11 @@ func TestApplyPlan_Replace_CtxCancelAfterDelete_SkipsCreate(t *testing.T) { }} result, err := ApplyPlanWithHooks(ctx, fp, plan, ApplyPlanHooks{}) if err != nil { - // ApplyPlan's loop check catches the cancellation at the next + // ApplyPlanWithHooks' loop check catches the cancellation at the next // iteration, but the per-action ctx check inside doReplace // fires first. Either path yields a per-action error rather // than a top-level error from this single-action plan. - t.Fatalf("ApplyPlan should not surface top-level error: %v", err) + t.Fatalf("ApplyPlanWithHooks should not surface top-level error: %v", err) } if fp.driver.deleteCount != 1 { t.Errorf("Delete should have run before cancellation; deleteCount=%d", fp.driver.deleteCount) diff --git a/plugin/external/proto/iac.pb.go b/plugin/external/proto/iac.pb.go index 29d737eb..944a2bec 100644 --- a/plugin/external/proto/iac.pb.go +++ b/plugin/external/proto/iac.pb.go @@ -2243,12 +2243,10 @@ type CapabilitiesResponse struct { // cutover (typedIaCAdapter previously couldn't ask the plugin for its // canonical-keys override). See ADR-0029. CanonicalKeys []string `protobuf:"bytes,2,rep,name=canonical_keys,json=canonicalKeys,proto3" json:"canonical_keys,omitempty"` - // compute_plan_version: provider-level apply-time dispatch version. - // "" or unrecognized = "v1" (legacy provider.Apply path); "v2" routes - // through wfctlhelpers.ApplyPlan. Mirrors the - // ComputePlanVersionDeclarer optional Go interface so plugin authors - // can declare apply-version via gRPC instead of a separate type-assert - // probe. See ADR-0029. + // compute_plan_version: provider-level apply-time lifecycle declaration. + // wfctl accepts "v2" only; empty, "v1", and unrecognized values are rejected + // by the provider loader after the strict lifecycle cutover. "v2" routes + // through wfctlhelpers.ApplyPlanWithHooks. See ADR-0029 and workflow#640. ComputePlanVersion string `protobuf:"bytes,3,opt,name=compute_plan_version,json=computePlanVersion,proto3" json:"compute_plan_version,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/plugin/external/proto/iac.proto b/plugin/external/proto/iac.proto index 5f1cdd61..cb23280b 100644 --- a/plugin/external/proto/iac.proto +++ b/plugin/external/proto/iac.proto @@ -399,12 +399,10 @@ message CapabilitiesResponse { // cutover (typedIaCAdapter previously couldn't ask the plugin for its // canonical-keys override). See ADR-0029. repeated string canonical_keys = 2; - // compute_plan_version: provider-level apply-time dispatch version. - // "" or unrecognized = "v1" (legacy provider.Apply path); "v2" routes - // through wfctlhelpers.ApplyPlan. Mirrors the - // ComputePlanVersionDeclarer optional Go interface so plugin authors - // can declare apply-version via gRPC instead of a separate type-assert - // probe. See ADR-0029. + // compute_plan_version: provider-level apply-time lifecycle declaration. + // wfctl accepts "v2" only; empty, "v1", and unrecognized values are rejected + // by the provider loader after the strict lifecycle cutover. "v2" routes + // through wfctlhelpers.ApplyPlanWithHooks. See ADR-0029 and workflow#640. string compute_plan_version = 3; } diff --git a/plugin/sdk/manifest.go b/plugin/sdk/manifest.go index b6c9c688..a6ec14da 100644 --- a/plugin/sdk/manifest.go +++ b/plugin/sdk/manifest.go @@ -1,10 +1,11 @@ // Package sdk hosts the plugin SDK manifest schema and helpers used by -// wfctl to discover plugin capabilities (currently: IaC dispatch version). +// wfctl to discover plugin capabilities. // // The SDK manifest is intentionally additive over [plugin.PluginManifest]; -// it captures only the fields that wfctl reads at apply-time to choose -// between the v1 (legacy in-provider Apply) and v2 (wfctlhelpers.ApplyPlan) -// dispatch paths. +// it captures only the fields that wfctl validates before typed runtime +// capability discovery. After the strict lifecycle cutover, the typed +// CapabilitiesResponse.compute_plan_version declaration is authoritative and +// wfctl accepts "v2" only. package sdk import ( @@ -50,13 +51,13 @@ type Manifest struct { // IaCProvider describes IaC-provider-specific manifest fields. type IaCProvider struct { - // ComputePlanVersion selects the apply-time dispatch path: - // "" (default, treated as "v1"): legacy in-provider Apply switch. - // "v1": explicit legacy dispatch. - // "v2": route through wfctlhelpers.ApplyPlan - // (Replace + input-drift postcondition). - // Schema-validated against the enum ["v1","v2"]; "" passes validation - // because the field is optional. + // ComputePlanVersion is parse-time manifest metadata retained for plugin + // authors and validation tooling. Runtime selection is strict: the typed + // CapabilitiesResponse.compute_plan_version gate accepts "v2" only and + // routes through wfctlhelpers.ApplyPlanWithHooks. + // Schema-validated against the enum ["v1","v2"]; "" passes validation for + // older manifests, but load-time typed capability validation rejects non-v2 + // providers. ComputePlanVersion string `json:"computePlanVersion,omitempty"` } diff --git a/plugin/sdk/manifest_schema.json b/plugin/sdk/manifest_schema.json index aec5a976..7dd7a527 100644 --- a/plugin/sdk/manifest_schema.json +++ b/plugin/sdk/manifest_schema.json @@ -15,7 +15,7 @@ "properties": { "computePlanVersion": { "type": "string", - "description": "Selects the apply-time dispatch path. v1 (default when omitted) routes through the legacy in-provider Apply switch. v2 routes through wfctlhelpers.ApplyPlan with full Replace + drift-postcondition support.", + "description": "Declares apply-time lifecycle support for validation metadata. Runtime typed capability discovery accepts v2 only and routes through wfctlhelpers.ApplyPlanWithHooks; empty/v1 declarations are rejected at provider load after the strict cutover.", "enum": ["v1", "v2"] } } From 3d9ffd7464e2127e68631df7972d84efb5918633 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 11:58:32 -0400 Subject: [PATCH 3/3] docs(iac): clarify manifest lifecycle metadata --- plugin/sdk/manifest_schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/sdk/manifest_schema.json b/plugin/sdk/manifest_schema.json index 7dd7a527..5118bf7d 100644 --- a/plugin/sdk/manifest_schema.json +++ b/plugin/sdk/manifest_schema.json @@ -15,7 +15,7 @@ "properties": { "computePlanVersion": { "type": "string", - "description": "Declares apply-time lifecycle support for validation metadata. Runtime typed capability discovery accepts v2 only and routes through wfctlhelpers.ApplyPlanWithHooks; empty/v1 declarations are rejected at provider load after the strict cutover.", + "description": "Declares apply-time lifecycle support for validation metadata. Runtime typed capability discovery, not this manifest field, is authoritative: CapabilitiesResponse.compute_plan_version must be v2 and routes through wfctlhelpers.ApplyPlanWithHooks. Older empty/v1 manifest declarations may parse but are not accepted as runtime capability evidence.", "enum": ["v1", "v2"] } }