Add optional IaC provider job runner contract#850
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new optional IaC provider “job runner” contract to the typed IaC plugin surface, and wires step.sandbox_exec to support a new exec_env: provider-ephemeral mode that delegates execution to the selected provider’s runner capability.
Changes:
- Introduces optional gRPC service
IaCProviderRunner(proto + generated bindings) and SDK auto-registration / contract registry advertisement. - Adds
interfaces.IaCProviderRunnerplus job handle/status/state types, and adds aniac/providerclientaccessor (Runner()) gated by advertised services. - Wires
step.sandbox_execexecution environmentprovider-ephemeralthrough a named IaC provider service’s runner accessor, and documents the new mode.
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wftest/bdd/strict_iac.go | Extends strict contract registration assertions to include IaCProviderRunner. |
| plugin/external/sdk/iacserver.go | Auto-registers IaCProviderRunner when implemented by a provider server. |
| plugin/external/sdk/contracts_logcapture_test.go | Adds test coverage ensuring the contract registry advertises the runner service. |
| plugin/external/proto/iac.proto | Adds IaCProviderRunner service and Job* messages/enums to the IaC proto. |
| plugin/external/proto/iac.pb.go | Regenerates protobuf Go types for the new runner service/messages. |
| plugin/external/proto/iac_proto_test.go | Extends optional-service interface distinctness test to include runner. |
| plugin/external/proto/iac_grpc.pb.go | Regenerates gRPC bindings for IaCProviderRunner. |
| plugin/external/adapter.go | Updates adapter documentation to include the new optional runner capability. |
| module/sandbox_remote_runners.go | Reserves provider-ephemeral so it can’t be shadowed by a remote runner name. |
| module/sandbox_remote_runners_test.go | Updates reserved-name test to include provider-ephemeral. |
| module/provider_ephemeral_runner.go | Implements a sandbox runner that executes via interfaces.IaCProviderRunner. |
| module/pipeline_step_sandbox_exec.go | Adds provider config and routes exec_env: provider-ephemeral through resolver. |
| module/execenv_factory.go | Adds provider-ephemeral resolver wiring that looks up a provider service runner accessor. |
| module/execenv_factory_test.go | Adds tests for provider-ephemeral resolution and a fake runner provider. |
| interfaces/iac_provider.go | Adds IaCProviderRunner, JobHandle, JobStatusReply, and JobState types. |
| interfaces/iac_provider_test.go | Adds tests verifying runner optional-interface satisfaction/non-satisfaction. |
| iac/providerclient/adapter.go | Adds advertised-service gated runner client adapter + proto/type conversions. |
| iac/providerclient/adapter_test.go | Adds round-trip tests for advertised/unadvertised runner capability behavior. |
| docs/WFCTL.md | Documents step.sandbox_exec execution environments including provider-ephemeral. |
| docs/AGENT_GUIDE.md | Notes core-vs-plugin boundary for the shared runner contract and plugin implementations. |
Files not reviewed (1)
- plugin/external/proto/iac_grpc.pb.go: Language not supported
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This was referenced Jun 3, 2026
intel352
added a commit
that referenced
this pull request
Jun 3, 2026
…nal providers (infra-admin PR13/13, ADR 0021) (#851) * feat(providerclient): wire ResourceDriver gRPC service into Adapter (PR-2) Implements the deferred PR-2 migration in iac/providerclient.Adapter: - Adds IaCServiceResourceDriver const ("workflow.plugin.external.iac.ResourceDriver") - Adds resourceDriverAdapter implementing interfaces.ResourceDriver (all 8 methods: Create/Read/Update/Delete/Diff/HealthCheck/Scale/SensitiveKeys) PLUS the optional interfaces.Troubleshooter (Troubleshoot) using the same JSON-bytes convention (config_json/outputs_json) as the existing typed adapter - Compile-time guards: var _ interfaces.ResourceDriver and var _ interfaces.Troubleshooter on *resourceDriverAdapter (mirror typedResourceDriver) - Wires advertisement-gated pb.ResourceDriverClient in New(); nil when not advertised - Replaces the ErrProviderMethodUnimplemented stub with a live implementation - Adds ResourceDriverProvider accessor interface mirroring RegionListerProvider/ RunnerProvider pattern; not-advertised path still returns ErrProviderMethodUnimplemented - mapResourceDriverGRPCError maps codes.NotFound/AlreadyExists/ResourceExhausted/ Unavailable/DeadlineExceeded/Unauthenticated/PermissionDenied/InvalidArgument/ FailedPrecondition/Unimplemented to engine sentinels. Wraps with %w/%w (sentinel + original gRPC error) so callers recover BOTH the sentinel (errors.Is) AND the gRPC status (status.Code walking the unwrap chain); default case keeps method attribution while preserving the status via %w Closes the runtime advertisement gap: plugin/external/adapter.go advertisedOptionalIaCServices() now forwards IaCServiceResourceDriver (and IaCServiceRunner, which #850 added but the switch never matched) from the plugin's ContractRegistry to providerclient.New, so the WiringHook-registered adapter constructs the real ResourceDriver client. Without this, the adapter stayed nil and ApplyPlanWithHooks -> provider.ResourceDriver(type) still hit ErrProviderMethodUnimplemented end-to-end. - Tests (providerclient): bufconn round-trip for all 8 CRUD methods + Troubleshoot; negative advertisement gate; table-driven gRPC error-mapping test covering all 10 status codes (asserts both the errors.Is sentinel AND status.Code recovery via the unwrap chain) - Tests (plugin/external): advertisedOptionalIaCServices forwards ResourceDriver (and Runner) when advertised / excludes when not; end-to-end WiringHook -> GetService -> ResourceDriver(type) returns the real bridge and Create round-trips WITH config_json echoed back to prove the Config JSON survives the boundary; unadvertised plugin still returns the unimplemented error Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(providerclient): Scale errors on out-of-int32 replicas + Unimplemented wraps gRPC err (Copilot) - Scale rejects out-of-int32-range replicas explicitly (was clampInt32 → silent saturation → unintended scale); mirrors typedResourceDriver.Scale. - mapResourceDriverGRPCError Unimplemented case wraps the original gRPC error (%w) so status.Code stays recoverable, consistent with the other mapped cases + translateRPCErr. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- 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.
Summary
Refs #840. This is the workflow-core slice; provider plugin implementations and registry capability flags remain follow-up work. #731 intentionally ignored per direction. #704 was rechecked against workflow-plugin-infra and left deferred.
Verification
RED before implementation:
GREEN after implementation:
Note:
buf lintis not a clean repo gate today; it reports pre-existing package/service naming violations across plugin/external/proto. The new enum zero value was adjusted to avoid adding the avoidable enum-zero lint violation.