feat(iac): step.iac_commit_back + step.iac_provider_reconcile (infra-admin P2 PR5/12)#845
Merged
Conversation
…fra-admin P2 PR5/12)
Task 9: step.iac_commit_back — serialises authored specs via iac/specgen.SpecToYAML and
commits back to git after a full-success apply (no errors + action_count match). Returns
{committed:false, reason:"partial-apply"} on partial apply; {state_diverged:true} (HTTP 207)
when apply succeeded but git failed. Supports branch-push and gh-pr targets. secret:// refs
survive verbatim. Injected GitExecFn pattern mirrors apply step.
Task 10: step.iac_provider_reconcile — drift → import → approximate cloud-snapshot YAML →
draft PR. Explicitly approximate (NOT via SpecToYAML); mandatory disclaimer header. Returns
{draft:true, warning:...} when drifted resources found; {draft:false} when no drift.
All 7 registration places completed: StepTypes manifest, StepFactories, DOCUMENTATION.md,
schema/schema.go (alphabetical), schema/module_schema.go, schema/step_schema_builtins.go,
plugins/platform/plugin_test.go. Golden file regenerated. go test ./... exit 0.
Code-review fixes (CHANGES_REQUESTED):
- [Critical] Prod gitExecFn now runs HOST-NATIVE via os/exec (was a per-call ephemeral
DockerSandbox that lost git working-tree state between commands and never mounted repo_dir,
so every git op failed → always state_diverged). Inherits os.Environ() so GH_TOKEN/
GITHUB_TOKEN are forwarded; no shell; full argv run directly. GitExecFn signature gains a
workDir parameter (the step's repo_dir); the sandbox stays for the remote-runner's
arbitrary commands in a later PR.
- [Critical] Both steps now build COMPLETE arg slices (git/gh as argv[0]) and pass them whole
to exec — no entrypoint double-prefix, no "git git push". Removed the runGit prefix-stripping
helpers. Uses git add -A.
- [Important] step.iac_provider_reconcile gains a target config (branch-push default | gh-pr);
branch-push pushes the draft branch, gh-pr calls gh pr create --draft. Git failure now
returns {draft:false, state_diverged:true, reason} (was a false draft:true with no PR).
- [Important] Added TestIaCProviderReconcile_GitFails_StateDiverged + TestIaCProviderReconcile_GhPRTarget.
- [Minor] Added TestIaCCommitBack_PartialApplyByCount_NoCommit (empty errors but
action_count > len(actions) → not full success) + workdir/full-argv assertions.
golangci-lint v2.12.0 --new-from-rev=origin/main ./module/ ./plugins/platform/: 0 issues.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds two new IaC pipeline steps to close the “direct apply → git drift” loop: (1) committing authored specs back to git only on full-success applies, and (2) reconciling detected drift by importing an approximate cloud snapshot and opening/pushing a draft for review.
Changes:
- Introduces
step.iac_commit_backto serialize authored specs viaiac/specgen.SpecToYAMLand commit/push (or open PR) only on full apply success. - Introduces
step.iac_provider_reconcileto detect drift, import approximate state snapshots, and publish a draft branch/PR with a mandatory warning. - Registers both steps across schema/docs/editor golden data and wires platform plugin factories + tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| schema/testdata/editor-schemas.golden.json | Adds editor schema entries for the two new step types. |
| schema/step_schema_builtins.go | Registers schemas/config/outputs for step.iac_commit_back and step.iac_provider_reconcile. |
| schema/schema.go | Adds new step types to coreModuleTypes. |
| schema/module_schema.go | Adds module schema descriptors for the two new steps. |
| plugins/platform/plugin.go | Adds host-native gitExecFn and wires both new step factories into the platform plugin. |
| plugins/platform/plugin_test.go | Updates expected platform step factory list to include the new steps. |
| module/pipeline_step_iac_provider_reconcile.go | Implements reconcile step: drift detection → import → YAML snapshot → draft publish. |
| module/pipeline_step_iac_provider_reconcile_test.go | Adds tests covering reconcile targets, warning text, and git-failure behavior. |
| module/pipeline_step_iac_commit_back.go | Implements commit-back step and “full success” gating based on apply output. |
| module/pipeline_step_iac_commit_back_test.go | Adds tests covering full success, partial apply, secret ref survival, gh-pr target, and git failures. |
| DOCUMENTATION.md | Documents the two new steps in the step catalog. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…ety, target validation, drift order, count doc)
- gitExecFn: dedupe env (build map from os.Environ, overrides win) so GH_TOKEN
override reliably takes effect (was duplicate KEY= entries).
- isFullSuccess: require action_count present + numeric — missing/non-numeric
no longer classifies as full success (closes commit-on-empty/garbage hazard).
- commit_back + reconcile: validate 'target' ∈ {branch-push, gh-pr}, factory
error on unknown (was silent default).
- reconcile: match DetectDrift results to refs by Name+Type (was positional
index → wrong ProviderID / panic on reordered/short results).
- count doc: 'drifted resources detected' (matches len(drifted)) in schema +
comment.
Tests: MissingActionCount/NonNumericActionCount_NoCommit, DriftOrderIndependent,
Factory_InvalidTarget (both steps) — the safety/panic ones proven to fail pre-fix.
Co-Authored-By: Claude Opus 4.8 (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.
PR5/12 —
step.iac_commit_back+step.iac_provider_reconcile(infra-admin Phase 2)Locked plan Tasks 9–10; ADR 0016. Closes the direct-apply loop: commit applied specs back to git (so git never drifts from state), and turn drift into an approximate review-required draft.
step.iac_commit_backChains after
step.iac_provider_apply. Commits only on full success (apply_result.errorsempty ANDaction_count == len(actions)) — a partial apply returns{committed:false, reason:"partial-apply"}with NO commit. Serializes the AUTHORED specs viaiac/specgen.SpecToYAML(sosecret://refs are preserved verbatim, never expanded), writes them, and runs git via an injectedGitExecFn. A commit failure AFTER a successful apply returns{state_diverged:true}(route → HTTP 207; idempotent).target:branch-push(default) |gh-pr.step.iac_provider_reconcileDrift →
provider.Import→ an approximate cloud-snapshot YAML (explicitly NOTSpecToYAML) → draft commit/PR carrying the mandatory warning "imported from cloud; approximate; does NOT reconstruct your secret:// refs — review before merge".targetbranch-push|gh-pr. Git failure →{draft:false, state_diverged:true}(never claims a draft that wasn't produced).Git execution — host-native (design backport)
The git/gh commands run host-native via
os/exec(arg slices, no shell;GH_TOKEN/GITHUB_TOKENforwarded from the engine env), NOT in a Docker sandbox. Review proved a per-command ephemeral sandbox can't persist git state or see the repo dir; the engine committing to its own repo with a fixed argv is not untrusted-code execution (thestandard-profile sandbox stays for the remote-runner's arbitrary commands in a later PR). ADR 0016 left the mechanism open. Recorded as a design backport.Review notes (resolved)
3 Criticals fixed: ephemeral-sandbox-per-call/no-mount → host-native;
git git pushdouble-prefix → full argv; missingGH_TOKEN→ forwarded. Reconcile gainedtarget+ branch-push fallback +draft:false-on-git-failure (was a contract lie). Added git-failure + partial-apply-by-count + gh-pr-target tests.Registered in all 7 places (factory+StepTypes, DOCUMENTATION.md, coreModuleTypes, module_schema.go, step_schema_builtins.go, editor golden, plugin_test expectedSteps). Verified:
go build ./...exit 0; fullgo test ./...exit 0 (149 ok);golangci-lint v2.12.0 --new-from-rev0 issues.🤖 Generated with Claude Code