feat: workflow#640 Phase 1 — v2 action lifecycle inventory + provider compatibility ADR + Deprecated marker#691
Merged
Conversation
…tibility expectations Phase 1 of workflow#640. Bounded docs-only deliverable: - Inventory v1 ApplyPlan callers (3 conformance scenarios + 3 wfctl provider.Apply direct calls + 4 plugin canonical-form delegates) - Classify each: KEEP-ON-V1 / MIGRATE-NEEDED / TOOL - Define 5 provider-compatibility expectations (per-action success evidence, failed-delete-no-prune, compensation evidence, update-failure- no-delete, provider-owned-replace-advertised) for Phase 2's gRPC contract Phase 2-5 (gRPC contract extension, plugin migration, conformance migration, v1 deprecation+removal) deferred to subsequent design passes. Self-challenge surfaced 3 doubts: docs-only-as-theatre risk (mitigated by ADR + in-package doc.go); inventory smaller than expected (Phase 2 is the heavy work; Phase 1 is prerequisite, not trivializing); 3 conformance scenarios "KEEP-ON-V1" preliminary (revisit during Phase 4). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Critical fixes: - C-1: REVOKE Assumption 5 (graceful proto fallback) — contradicts ADR 0024 (no-compat-shim mandate). Phase 2 is now explicitly a HARD-CUTOVER coordinated PR cascade across workflow + 4 plugins, recorded as ADR 0040 consequence. - C-2: Reclassify Item #9 (typed_adapter.Apply) — NOT in v2 hot path; v2 dispatch via ResourceDriver per action; Phase 2 wire-format work happens in applyResultFromPB decoder, not adapter dispatch. Important fixes: - I-1: Reclassify 3 conformance scenarios from KEEP-ON-V1 to MIGRATE-NEEDED (Phase 4) — TRIVIAL empty-hooks rename; HARD PREREQUISITE for Phase 5 documented. - I-2: Add Task 0 (PRE-PLAN-AUTHORING grep across 4 plugin repos) before ADR commits — eliminates speculative-inventory bootstrap problem. - I-3: Add Assumption 8 (manifest-validation gap on deploy_providers path) — Phase 2 scope adds validation gate. Scope expansion (per cycle-1 reviewer's "Option A"): - Add // Deprecated: godoc marker to ApplyPlan in iac/wfctlhelpers/apply.go - Bump iac-codemod applyCanonicalCallExpr to ApplyPlanWithHooks form (codemod becomes Phase 3 migration driver) - Update lint matchers + doc references consistently Phase 1 ships engine docs + deprecation marker + codemod canonical-form bump. Light mechanical changes; no runtime behavior change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…verride per autonomous mandate) Cycle 2 adversarial review surfaced 2 Critical findings — both new from the cycle-1 Option A scope expansion: C-1: Inventory table "KEEP for now" contradicted Goal item 6 "bump now". Resolved by removing Goal item 6 (codemod constant bump) from Phase 1 scope; deferring to Phase 3 lockstep with AST-rewriter updates. C-2: Codemod constant bump alone is theatre — applyCanonicalCallExpr is //nolint:unused, NOT consumed by rewriteApplyBody. Bumping the string without updating rewriteApplyBody/isAlreadyDelegatedApplyBody/ runAssertApplyDelegatesToHelper produces internally-inconsistent tool (constant says WithHooks, AST emits ApplyPlan, idempotency gate recognizes ApplyPlan only, lint flags WithHooks callers as non-canonical, regression-loop on re-run). Resolved by removing codemod constant bump from Phase 1 entirely; Phase 3 does it in lockstep when codemod actually runs against plugins. Plus Important fixes: - I-2: applyPlanWithEnvProvider Deprecated marker scope clarified — unexported function gets NO marker (gopls/staticcheck ignore unexported Deprecated identifiers). Only exported ApplyPlan gets the marker. - I-3: Phase 2 constraint reference subsection added — explicitly directs Phase 2 writing-plans agent to read ADR 0040 first. Per skill spec at 2-revision-cycle limit; user-override per "continue autonomously" mandate. Surgical fix; not full re-review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… compatibility ADR + Deprecated marker Phase 1 deliverable per docs/plans/2026-05-16-v2-lifecycle-phase1-design.md: - docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md — full v1 caller inventory (3 conformance scenarios + 3 wfctl provider.Apply direct calls + 4 plugin Apply impls) verified via per-repo gh api grep - decisions/0040-v2-action-lifecycle-provider-compatibility.md — ADR recording 5 provider-side compatibility expectations + Phase 2 hard-cutover constraint per ADR 0024 - iac/wfctlhelpers/doc.go — in-package contract pointer documenting v1/v2 versions + per-plugin migration path - iac/wfctlhelpers/apply.go:78 — // Deprecated: use ApplyPlanWithHooks godoc marker on ApplyPlan; surfaces via gopls/staticcheck to all callers MAJOR ARCHITECTURAL FINDING (verified inline 2026-05-16): - workflow-plugin-aws AWSProvider.Apply (provider/provider.go:237) — NON-CANONICAL - workflow-plugin-gcp GCPProvider.Apply (provider/provider.go:226) — NON-CANONICAL - workflow-plugin-azure AzureProvider.Apply (internal/provider.go:138) — NON-CANONICAL - workflow-plugin-digitalocean DOProvider.Apply (internal/provider.go:274) — CANONICAL ✓ 3 of 4 plugins do NOT use the iac-codemod canonical pattern. Phase 3 plugin migration is bifurcated: codemod for DO, manual for aws/gcp/azure. Phase 2 v2 contract design must accommodate both implementation paths. Cycle 1+2 adversarial-design-review findings addressed (cycle-2 polish per autonomous mandate; codemod constant bump moved from Phase 1 to Phase 3 lockstep with AST-rewriter updates after cycle-2 found bumping just the constant produces internally-inconsistent tool). Phases 2-5 deferred to subsequent design passes per Phase 1 scope. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Phase 1 of workflow#640: documents the v2 IaC action lifecycle migration plan/constraints, inventories current v1 callers (including plugin-side patterns), and adds a Go deprecation signal for the legacy wfctlhelpers.ApplyPlan entrypoint.
Changes:
- Added Phase 1 inventory + migration writeups (design doc + migration inventory) and recorded provider compatibility expectations in ADR 0040.
- Introduced
iac/wfctlhelpers/doc.goto document v1/v2 entrypoints and migration phases for provider authors. - Marked
iac/wfctlhelpers.ApplyPlanas deprecated in Go doc sogopls/staticchecksurfaces migration guidance to callers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| iac/wfctlhelpers/doc.go | New package documentation describing v1/v2 apply entrypoints and migration guidance. |
| iac/wfctlhelpers/apply.go | Adds // Deprecated: marker to exported ApplyPlan. |
| docs/plans/2026-05-16-v2-lifecycle-phase1-design.md | Phase 1 design doc capturing scope, inventory, and constraints. |
| docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md | Phase 1 inventory of v1 caller sites (workflow + plugins) and implications. |
| decisions/0040-v2-action-lifecycle-provider-compatibility.md | ADR defining provider compatibility expectations and Phase 2 hard-cutover constraint. |
| @@ -0,0 +1,53 @@ | |||
| // Package wfctlhelpers exposes the IaC plan-execution engine that wfctl and | |||
Comment on lines
+14
to
+17
| 3. Defines provider compatibility expectations for v2 — what plugins must do at the gRPC IaCProvider.Apply boundary so wfctl-side hooks fire correctly. | ||
| 4. Lands ADR 0040 recording the per-caller classification, the provider-side expectation contract, **and the consequence that Phase 2 is a coordinated hard-cutover per ADR 0024 (no compat-shim path).** | ||
| 5. **Adds `// Deprecated: use ApplyPlanWithHooks` godoc marker to `ApplyPlan` in `iac/wfctlhelpers/apply.go:78`** — surfaced by `gopls`/`staticcheck` to every caller; mechanically delivers #640's "Add deprecation warnings" milestone. | ||
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Lint blocker (4 SA1019 staticcheck deprecation hits — proves the godoc
marker is working):
- cmd/wfctl/infra_apply_in_process_test.go:77 (was missing from inventory)
- iac/conformance/scenario_delete_action.go:74
- iac/conformance/scenario_replace_cascade_preserves_dependents.go:92
- iac/conformance/scenario_upsert_on_already_exists.go:88
Folded the trivial Phase 4 mechanical migration (4 single-line renames
ApplyPlan → ApplyPlanWithHooks(..., ApplyPlanHooks{})) into this Phase 1
PR. Separate followup PR for 4 single-line renames would be silly.
Copilot finding 1: doc.go duplicate package comment — DELETED doc.go;
extended apply.go's existing package-level comment with the v1/v2
contract pointer paragraph (single source of truth).
Copilot finding 2: design doc hard-coded line:78 reference will drift —
acknowledged but not changed (file:line will rot, but the godoc marker
itself is symbol-anchored; design doc is an artifact-of-record from the
specific commit, not a long-lived contract).
Verification:
- GOWORK=off go build ./... PASS
- GOWORK=off go vet ./iac/wfctlhelpers/ ./iac/conformance/ ./cmd/wfctl/ PASS
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ted in-progress.jsonl Internal superpowers state file slipped in via git add -A in the prior fixup commit. Add .claude/ to .gitignore + remove from index. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md:25
- This section is titled “v1 wfctlhelpers.ApplyPlan callers” and the table entries are described as needing a Phase 4 rename, but the referenced conformance files now call ApplyPlanWithHooks. Please update the heading/table/classifications to match the post-change reality (e.g., note they’ve already migrated and ApplyPlan has no remaining in-repo callers outside wfctlhelpers’ own tests).
### v1 `wfctlhelpers.ApplyPlan` callers (production)
| File:Line | Classification | Notes |
|-----------|----------------|-------|
| `iac/conformance/scenario_upsert_on_already_exists.go:88` | MIGRATE-NEEDED (Phase 4) — TRIVIAL | Conformance scenario; `ApplyPlanWithHooks(..., ApplyPlanHooks{})` rename suffices. **Phase 5 prerequisite** — Phase 5 removes ApplyPlan, so Phase 4 must precede. |
docs/plans/2026-05-16-v2-lifecycle-phase1-design.md:36
- The artifact list references
iac/wfctlhelpers/doc.goand hard-codesiac/wfctlhelpers/apply.go:78, but there is nodoc.gofile in-tree and the ApplyPlan declaration/Deprecated marker aren’t at that line anymore. Consider either adding the missing doc.go (if intended) and/or avoiding line-number pinning in docs (prefer symbol names/links) to keep these references stable.
Single-repo (workflow), 1-PR deliverable. Four artifacts:
- `docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md` — the inventory document (per-caller table + classification rationale)
- `decisions/0040-v2-action-lifecycle-provider-compatibility.md` — ADR recording: (a) the provider-side compatibility contract; (b) the explicit consequence that Phase 2 is a coordinated hard-cutover per ADR 0024
- `iac/wfctlhelpers/doc.go` — Go doc.go file embedding a SHORT version of the inventory + provider-expectation pointer to the migration doc
- **`iac/wfctlhelpers/apply.go:78` — add `// Deprecated: use ApplyPlanWithHooks` godoc to the EXPORTED `ApplyPlan` function only** (the `applyPlanWithEnvProvider` and `applyPlanWithEnvProviderAndHooks` helpers are unexported — `gopls`/`staticcheck` ignore Deprecated markers on unexported identifiers, so they get NO marker)
Comment on lines
+13
to
+17
| - **Phase 1 (this document)**: inventory + provider-compatibility ADR + Deprecated marker | ||
| - **Phase 2**: design + ship v2-hooks-over-gRPC contract (HARD-CUTOVER per ADR 0024) | ||
| - **Phase 3**: migrate plugins to v2 contract (per-plugin manual + codemod for canonical-form plugins) | ||
| - **Phase 4**: migrate 3 conformance scenarios from `ApplyPlan` to `ApplyPlanWithHooks` | ||
| - **Phase 5**: remove `wfctlhelpers.ApplyPlan` entirely |
| - 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 (with `// Deprecated:` marker on `ApplyPlan` per Phase 1) | ||
| - `iac/wfctlhelpers/doc.go` — in-package contract pointer |
Comment on lines
+18
to
+22
| **Explicitly out of scope for Phase 1** (deferred to Phase 2-5 design passes): | ||
| - Phase 2: design + ship the v2-hooks-over-gRPC contract — must be a HARD-CUTOVER coordinated PR cascade across workflow + 4 plugin repos per ADR 0024 (no compat shim, no graceful fallback) | ||
| - Phase 3: migrate plugins to emit hooks-aware Apply responses (per-repo PRs). **Phase 3 also bumps `iac-codemod`'s `applyCanonicalCallExpr` constant + updates `rewriteApplyBody` + `isAlreadyDelegatedApplyBody` + `runAssertApplyDelegatesToHelper` AST functions in lockstep** so the codemod becomes the migration driver. (Cycle-2 review correctly flagged that bumping ONLY the constant in Phase 1 is theatre — constant is `//nolint:unused`, not consumed by rewriter; risks leaving codemod internally inconsistent.) | ||
| - Phase 4: migrate the 3 conformance scenarios from `ApplyPlan(...)` to `ApplyPlanWithHooks(..., ApplyPlanHooks{})` (trivial mechanical rename; HARD PREREQUISITE for Phase 5 since Phase 5 removes ApplyPlan) | ||
| - Phase 5: remove `wfctlhelpers.ApplyPlan` entirely (after Phase 4 + plugin canonical-form propagation) |
Comment on lines
+48
to
+50
| - **Phase 4 conformance scenario migration is a trivial 6-line PR** (rename 3 call sites). HARD prerequisite for Phase 5. | ||
|
|
||
| - **Phase 5 ApplyPlan removal** gates on Phase 4 + plugin canonical-form propagation (DO already canonical; aws/gcp/azure must migrate to either canonical OR Phase-2-direct path). |
…ed into Phase 1 5 Copilot findings — all doc-consistency about Phase 4 status: - Design doc still listed Phase 4 as out-of-scope; updated to SHIPPED - Inventory doc claimed Phase 4 future work; updated to SHIPPED - Inventory doc referenced deleted iac/wfctlhelpers/doc.go; removed - ADR 0040 said Phase 4 future trivial 6-line PR; updated to SHIPPED - Phase 5 gating updated: was Phase 4 prerequisite, now only Phase 2+3 Inventory now lists 4 migrated callers (3 conformance + 1 test file). Symbol references (iac/wfctlhelpers.ApplyPlan) replace file:line where the design doc was hard-coding apply.go:78. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e-work refs with shipped status 5 remaining stale refs in design doc: - apply.go:78 hard-coded → iac/wfctlhelpers.ApplyPlan symbol - 3 conformance row classifications → MIGRATED in this PR - Self-challenge doubt 2 → updated to reflect mid-execution fold-in - Out-of-scope Phase 4 → struck through SHIPPED - Phase 5 gating → references Phase 2+3 (Phase 4 already shipped) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
+34
to
+38
| Single-repo (workflow), 1-PR deliverable. Four artifacts: | ||
| - `docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md` — the inventory document (per-caller table + classification rationale) | ||
| - `decisions/0040-v2-action-lifecycle-provider-compatibility.md` — ADR recording: (a) the provider-side compatibility contract; (b) the explicit consequence that Phase 2 is a coordinated hard-cutover per ADR 0024 | ||
| - `iac/wfctlhelpers/doc.go` — Go doc.go file embedding a SHORT version of the inventory + provider-expectation pointer to the migration doc | ||
| - **`iac/wfctlhelpers.ApplyPlan` symbol — add `// Deprecated: use ApplyPlanWithHooks` godoc** (grep `^func ApplyPlan` in `iac/wfctlhelpers/apply.go`). The EXPORTED `ApplyPlan` function only — `applyPlanWithEnvProvider` + `applyPlanWithEnvProviderAndHooks` helpers are unexported and `gopls`/`staticcheck` ignore Deprecated markers on unexported identifiers. |
Comment on lines
+80
to
+81
| - Phase 4 conformance scenario migration — separate trivial PR | ||
| - Phase 5 ApplyPlan removal — gates on Phase 4 completion + plugin canonical-form propagation |
7 tasks
intel352
added a commit
that referenced
this pull request
May 20, 2026
#743) * feat(iac): workflow#640 Phase 5 — remove legacy wfctlhelpers.ApplyPlan 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) <noreply@anthropic.com> * docs(iac): close v2 lifecycle migration notes * docs(iac): clarify manifest lifecycle metadata --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of workflow#640. Bounded docs + 1-godoc-line deliverable:
docs/migrations/2026-05-16-v2-lifecycle-phase1-inventory.md— full v1 caller inventory verified via per-repogh apigrepdecisions/0040-v2-action-lifecycle-provider-compatibility.md— ADR recording 5 provider-side compatibility expectations + Phase 2 hard-cutover constraint per ADR 0024 (no compat shim)iac/wfctlhelpers/doc.go— in-package contract pointeriac/wfctlhelpers/apply.go:78—// Deprecated: use ApplyPlanWithHooksgodoc marker onApplyPlan; surfaces viagopls/staticcheckto all callersMajor architectural finding (verified inline)
3 of 4 IaC plugins do NOT use the iac-codemod canonical pattern:
provider/provider.go:237AWSProvider.Applyprovider/provider.go:226GCPProvider.Applyinternal/provider.go:138AzureProvider.Applyinternal/provider.go:274DOProvider.Applywfctlhelpers.ApplyPlanPhase 3 plugin migration is bifurcated: codemod for DO; manual for aws/gcp/azure. Phase 2 v2 contract design must accommodate both implementation paths.
Pipeline
Out of scope (deferred to Phase 2-5 design passes per Phase 1 scope)
wfctlhelpers.ApplyPlanremovalTest plan
GOWORK=off go build ./iac/wfctlhelpers/PASSpackage wfctlhelpersdocumentation only; no runtime changegopls/staticcheckwill surface deprecation to ApplyPlan callers (3 conformance scenarios + DO plugin); operators see migration signal via static analysis🤖 Generated with Claude Code