feat(iac/admin): host-side infra.admin module + handler library + wfctl CLI#791
Merged
Merged
Conversation
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 1: introduce a public ResolveStateStore in iac/wfctlhelpers so the upcoming host-side infra.admin module (workflow/module/infra_admin.go) and the existing wfctl CLI subcommands share one implementation. Returns a full interfaces.IaCStateStore; out-of-subset methods (Lock/SavePlan/GetPlan) panic per design doc cycle-5 row 4 to surface unexpected callers loudly. cmd/wfctl/infra_state_store.go's resolveStateStore now delegates to wfctlhelpers.ResolveStateStore. isNoopStateStore recognises both concrete noop types (cmd/wfctl-side *noopStateStore + new *wfctlhelpers.NoopStateStore) so downstream "skip metadata persist" short-circuits stay honest. resolvePostgresStateStore + postgresWfctlStateStore removed (no production or test callers remained); plugin-served + filesystem helpers stay in cmd/wfctl because existing tests directly instantiate them. TestResolveStateStore_ReturnsDiscoverErrors updated to match the new config-load context (the failure now surfaces at config.LoadFromFile rather than the discover wrapper; user-facing diagnosis is equivalent). Verified: GOWORK=off go test ./cmd/wfctl/... ./iac/... ./module/... ./plugin/... + golangci-lint --new-from-rev=origin/main all green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…a/container type predicates (spec F2) Spec-reviewer F2 on 7a064b8 flagged that writeEnvResolvedConfig, isInfraType, and isContainerType existed as byte-identical duplicates in both cmd/wfctl and iac/wfctlhelpers after the Task-1 lift — the commit message claimed delegation but env_resolve was left forked. Path (a) chosen: export the wfctlhelpers symbols and reduce the cmd/wfctl versions to one-line delegating shims so existing cmd/wfctl callsites keep compiling unchanged but cannot drift from the shared helper. - WriteEnvResolvedConfig / IsInfraType / IsContainerType exported from iac/wfctlhelpers/env_resolve.go. - cmd/wfctl/infra_env_resolve.go reduces to a one-line wrapper. - cmd/wfctl/infra.go isInfraType + isContainerType become one-line wrappers. - iac/wfctlhelpers/state.go ResolveStateStore docstring expanded with the three-step pluginDir fallback order (spec F1 — host-module handoff aid noting that empty pluginDir lets the WFCTL_PLUGIN_DIR env var configure both CLI and host module via a single knob). Verified: GOWORK=off go test ./iac/wfctlhelpers/... ./cmd/wfctl/ green; golangci-lint --new-from-rev=origin/main -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… plugin loader seam; fix sanitizeStateID divergence (code-review I-1..I-3, M-1..M-3) Addresses code-reviewer findings on commit 7a064b8: I-1 (consolidation finish) — already shipped in a9d66e3 (isContainerType → 1-line shim alongside isInfraType + writeEnvResolvedConfig). I-2.1 — 9-case table-driven panic test in iac/wfctlhelpers/state_invariants_test.go (3 stores × {SavePlan, GetPlan, Lock}) asserts panic with `wfctlhelpers:` prefix + method name + the "out-of-subset" rationale string. Guards design-doc cycle-5 row 4 so a future refactor that returns nil-error stubs is loud rather than silent. I-2.2 — cmd/wfctl/infra_noop_detection_test.go covers isNoopStateStore for both *noopStateStore (legacy) and *wfctlhelpers.NoopStateStore (post-lift) returning true, plus *fsWfctlStateStore returning false. The check feeds the post-apply "skip metadata persist when no-op" short-circuit; if a concrete type goes unrecognised, real state is silently corrupted by metadata.json from a discarded apply. I-2.3 — iac/wfctlhelpers/state_plugin_internal_test.go (white-box, package wfctlhelpers) exercises the spaces/s3/gcs branch via the loadPluginStateBackendClients seam with a fake pb.IaCStateBackendClient. Two cases: configures-advertised-backend (digitalocean priority, JSON config plumbing, ListResources round-trip) + no-advertising-plugin error context (names both the directory and the backend). I-3 — TestResolveStateStore_EnvOverride writes a config with environments.staging overriding the state directory, pre-stages a fixture in the staging dir, and asserts ResolveStateStore(cfg, "staging", "") returns a store that lists the staging fixture. Verifies temp file cleanup. TestResolveStateStore_EnvOverride_PropagatesError confirms the envName context is preserved on load failure. M-1 — godoc on loadPluginStateBackendClients spells out the test-seam contract ("Production callers MUST NOT mutate it.") M-2 — moduleStoreAdapter.mu removed; replaced with closed bool. Lock was load-bearing for nothing (Close already nil'd mgr); the closed flag documents the double-Close-is-safe invariant without the zero-work mutex. sync import dropped. M-3 (real bug fix, not cleanup) — SanitizeStateID lifted to iac/wfctlhelpers/state.go with cmd/wfctl's byte-exact algorithm (4-char replacer, not the allowlist my first draft used — the divergence would have broken cross-path mutual readability for resource names containing spaces, '@', '+', '#', etc.). cmd/wfctl/infra_state.go's sanitizeStateID reduces to a one-line shim. Verified: - GOWORK=off go test ./iac/wfctlhelpers/... ./cmd/wfctl/ green (157s). - 14 new test cases (9 panic + 2 env-override + 2 plugin loader + 3 noop-detection subtests) all pass. - golangci-lint --new-from-rev=origin/main -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ule+CLI use Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 2: introduce a public LoadIaCProviderFromConfig in iac/wfctlhelpers so the existing wfctl bootstrap path and the upcoming `wfctl infra admin` CLI subcommands (T19-T20) share one definition. Plan-deviation note (justified, called out for spec-reviewer): - Plan declared the 2-arg signature `LoadIaCProviderFromConfig(ctx, cfgFile)`. - cmd/wfctl's loader chain (discoverAndLoadIaCProvider -> typedIaCAdapter -> buildTypedIaCAdapterFrom -> enforceCapabilitiesV2Gate -> ...) is ~2800 lines of plugin-manager + gRPC-adapter machinery; lifting that wholesale into wfctlhelpers was out of scope for Task 2. - Resolution: keep the 2-arg signature exactly; add a package-level `Resolver IaCProviderResolverFunc` seam plus an `UnregisteredResolver` safe default that returns a clear error (no nil-func panics). - cmd/wfctl registers its real loader via `cmd/wfctl/provider_resolver_init.go::init()` so production wiring happens at package load time without any new public surface. - The host-side infra.admin module (T15) does NOT call this function; it resolves providers via app.GetService(<module>) per the modular DI graph. The seam therefore principally serves wfctl's CLI codepaths today. Behavior preserved: - Returns (nil, nil, nil) — not an error — when no iac.provider module is declared (matches the previous cmd/wfctl behavior). - First-match-wins module selection (still single-provider; multi arrives in Task 3 via LoadAllIaCProvidersFromConfig per design cycle-4 Important #6). - config.ExpandEnvInMap applied before provider-type extraction so ${VAR} references resolve at load time as before. cmd/wfctl/infra_bootstrap.go:loadIaCProviderFromConfig reduces to a one-line shim wrapping the helper output back into the local anonymous io.Closer return type so existing callers compile unchanged. Tests added (TDD): - TestLoadIaCProviderFromConfig_StubProvider — happy path with a fake Resolver returning an in-process stubProvider; asserts provider Name + closer non-nil + Resolver invocation count. - TestLoadIaCProviderFromConfig_NoProviderModule — returns (nil, nil, nil) when no iac.provider module exists. - TestLoadIaCProviderFromConfig_FirstMatchWins — guards the first-match invariant (Task 3 will introduce multi-provider). - TestLoadIaCProviderFromConfig_LoadError — config-load error surfaces with context. - TestLoadIaCProviderFromConfig_NoResolverRegistered — verifies UnregisteredResolver default returns a clear error rather than a nil-func panic. Verified: - GOWORK=off go test ./iac/wfctlhelpers/... ./cmd/wfctl/ green (176s). - golangci-lint --new-from-rev=origin/main -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…helper Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 3 + design-doc cycle-4 Important #6: LoadIaCProviderFromConfig is first-match-only, which is correct for the wfctl single-cloud bootstrap path but insufficient for the admin-UI handler library (T5/T6) that lists every configured provider with module-name attribution. New helper iterates all iac.provider modules and returns them keyed by module name. Signature (matches plan §Task 3 verbatim): LoadAllIaCProvidersFromConfig(ctx, cfgFile) -> (map[string]interfaces.IaCProvider, []io.Closer, error) Design choices: - Closer slice carries one entry per resolved provider in declaration order so callers `defer c.Close()` over all entries. - Resolver-failure rollback: on the Nth resolver error, every previously-resolved provider has Close() called (best-effort) before the helper returns (nil, nil, err). Otherwise an error from provider #3 would leak the subprocesses + plugin managers of providers #1 and #2 — callers have no handle to release them. - iac.provider modules missing a `provider:` string field are silently skipped (consistent with LoadIaCProviderFromConfig's single-module shape; misconfigured modules don't fail the whole load). - Both helpers now route through a new private loadProviderModule() per-module loader so the body cannot drift between the single and multi paths — addresses the cycle-4 reviewer's first-match-bug-risk observation by making the loader logic single-sourced. Tests added (TDD): - TestLoadAllIaCProvidersFromConfig_Two — plan §Step 1 minimum: two iac.provider modules return as map[stub-a, stub-b] with 2 closers. - TestLoadAllIaCProvidersFromConfig_EmptyConfig — config without any iac.provider yields empty map + nil closers + nil error. - TestLoadAllIaCProvidersFromConfig_SkipsMissingProviderField — mixed config (one valid + one missing `provider:` field) returns only the valid entry; Resolver is invoked exactly once. - TestLoadAllIaCProvidersFromConfig_ResolverErrorRollsBack — when provider #3 fails to resolve, providers #1 and #2 are closed before the error returns and the result is (nil, nil, err). Verified: - GOWORK=off go test ./iac/wfctlhelpers/... green. - GOWORK=off go build ./... clean. - golangci-lint --new-from-rev=origin/main -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…init() wiring (code-review I-1..I-3, M-1) Addresses code-reviewer findings on commit 63129d6: I-1 (TestLoadIaCProviderFromConfig_ExpandsEnvInModuleConfig) — guards that config.ExpandEnvInMap runs against mod.Config BEFORE the Resolver dispatch. Setenv WFCTLHELPERS_TEST_REGION + WFCTLHELPERS_TEST_TOKEN, write ${VAR} refs in the YAML, capture the cfg map the fake Resolver receives, assert the literal values flowed. Env-var expansion is a known regression footgun in this codebase (MEMORY.md BMW os.ExpandEnv 9-layer-bug-chain); this test pins the load-time expansion contract so a future "move expansion downstream" refactor fails loudly. I-2 (TestLoadIaCProviderFromConfig_SkipsEmptyProviderField) — covers the previously-uncovered skip-and-continue branch. Module A has `config: {}` (no provider field), module B has `provider: beta` → assert provider.Name()=="beta" + Resolver called exactly once with "beta". Without this test, a "fail-fast on missing provider field" refactor could silently break first-match-after-skip semantics for configs with a typo'd module followed by a valid one. I-3 (cmd/wfctl/provider_resolver_init_test.go) — function-pointer comparison asserts wfctlhelpers.Resolver is NOT the UnregisteredResolver default after package init. If provider_resolver_init.go is deleted or its init() breaks, every `wfctl infra apply` returns the unregistered-resolver error — a graceful failure but only discoverable by running a real command. This test catches it at `go test`. M-1 — godoc tightening on wfctlhelpers.Resolver explicitly stating the contract ("Production callers other than cmd/wfctl's init() MUST NOT mutate this var; tests substitute fakes with t.Cleanup restore. NOT goroutine-safe."). Matches the T1 loadPluginStateBackendClients precedent (dd4a427 M-1 follow-up). Option 2 chosen — adding an exported RegisterResolver() setter would diverge from T1's pattern without strengthening the actual guarantee. M-2 — folded into I-1: the new test captures the full cfg map the fake Resolver received and asserts flow-through (provider type + region + token), so no separate "cfg flowed at all" test is needed. M-3 — declined: the installFakeResolver helper's *[]string return form is test-helper-idiomatic; reviewer marked as leave-as-is. Verified: - GOWORK=off go test ./iac/wfctlhelpers/... ./cmd/wfctl/ green. - 7 wfctlhelpers tests + 1 cmd/wfctl init-wiring test all pass. - golangci-lint --new-from-rev=origin/main -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…comments (T3 code-review M-1, M-3) Doc-only follow-up to T3 commit 9dff952 addressing code-reviewer's optional Minors: M-1 — Added explicit invariant comment on LoadAllIaCProvidersFromConfig noting that cfg.Modules has unique Names (enforced upstream by config.LoadFromFile). The map-keyed-by-Name design silently overwrites on duplicate names while still releasing the earlier closer via the caller's slice — acceptable today but documents the load-bearing uniqueness assumption for future readers. M-3 — Added explanatory comment on the rollback-path `_ = c.Close()` line explaining the intentional swallow: the primary Resolver error takes precedence; surfacing a cleanup error would mask the root cause. M-2 was a transitive concern of T2 M-1 (Resolver-export goroutine safety) and was already addressed via the godoc tightening on wfctlhelpers.Resolver in commit ecafd76. No behavior change. Verified: go build ./... clean, go test ./iac/wfctlhelpers/... green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 4: proto file
+ generated Go bindings for the host-side infra.admin module's typed
HTTP surface. Sole wire format is protojson over HTTP. v1 is
read-only.
Package: workflow.iac.v1
Go package: github.com/GoCodeAlone/workflow/iac/admin/proto;adminpb
Service shape (5 RPCs):
- ListResources(AdminListResourcesInput) -> AdminListResourcesOutput
- GetResource(AdminGetResourceInput) -> AdminGetResourceOutput
- ListResourceTypes(AdminListResourceTypesInput) -> ...Output
- ListProviders(AdminListProvidersInput) -> AdminListProvidersOutput
- GenerateConfig(AdminGenerateConfigInput) -> ...Output
The HTTP audit-tail endpoint (GET /api/infra-admin/audit) streams
ndjson of AdminAuditEntry OUTSIDE this gRPC service per design doc
§Access logging — no AuditTail RPC. The plan task description noted
"6 RPC services" which appears to be off-by-one against the
design's 5-RPC InfraAdminService block; the design is authoritative.
Hard invariants encoded in the proto comments:
- Every typed input carries AdminAuthzEvidence; read endpoints
default-deny without evidence.authz_checked && authz_allowed.
- Free-form per-resource AppliedConfig / Outputs payloads cross
the wire as `bytes <name>_json`; the handler owns the
serialization shape. Same pattern as plugin/external/proto/iac.proto.
- error field uses tag 100 as the uniform discriminator across
output messages.
Tests added (TDD):
- TestAdminListResourcesInput_Roundtrip — plan §Step 3 smoke test;
protojson round-trips scalar + nested authz evidence.
- TestAdminResourceDetail_Roundtrip — pins that bytes-shaped
applied_config_json + outputs_json fields survive protojson without
base64-misinterpretation; covers SensitiveOutputsRedacted slice.
- TestAdminGenerateConfigInput_FieldValuesMap — protojson map<string,
string> handling for the form-builder submission.
- TestAdminListResourcesOutput_ErrorField — discriminator tag-100
convention pinned for generic decoder sniffing.
Generated via:
protoc --go_out=. --go_opt=paths=source_relative \
iac/admin/proto/infra_admin.proto
(protoc 35.0, protoc-gen-go v1.36.11)
Verified:
- GOWORK=off go test ./iac/admin/proto/... green (4 tests).
- GOWORK=off go build ./... clean.
- golangci-lint --new-from-rev=origin/main -> 0 issues.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 7a: package
skeleton for the host-side FieldSpec catalog that drives the
new-resource form-builder UI and feeds AdminFieldSpec entries on
InfraAdminService.ListResourceTypes (handler library T5/T6).
T7a split out of T7 per plan-adversarial C1: Lane A's T5 handler
library imports *catalog.FieldSpecCatalog as a typed parameter, so
the package + type + New() must exist BEFORE T5 compiles. This
skeleton makes T5/T6 buildable while T7b fills the 13 typed-Config
entries in catalog/fields.go in parallel after T7a lands.
Skeleton API (all from plan §Task 7a):
- FieldSpec struct — mirrors workflow.iac.v1.AdminFieldSpec
field-for-field with full godoc per field (Name/Label/Kind/
Required/EnumValues/EnumSource/Description/DefaultValue/Sensitive/
ElementKind/MinCount/MaxCount/DependsOnField).
- FieldSpecCatalog struct + New() returning an empty catalog.
- (*FieldSpecCatalog).Get(typeName) ([]FieldSpec, bool) — defensive
copy so callers cannot mutate internal state; returns
(nil, false) on unknown type so callers can distinguish missing
type from registered-but-empty.
- (*FieldSpecCatalog).AllTypes() []string — sorted; deterministic
ordering for snapshot tests + diff-friendly downstream consumers.
- catalog.FreeformReason(typeName, fieldName) (string, bool) —
package-level function exactly per the T7b audit-test signature
in plan §Step 3.
Seams for T7b:
- `var catalogEntries = func() map[string][]FieldSpec {...}` —
package-level var T7b's fields.go can replace via direct
assignment without touching catalog.go.
- `var freeformReasons = map[string]map[string]string{}` — parallel
annotation table T7b populates alongside string-kind entries.
Tests added (TDD; plan §Task 7a smoke):
- TestNew_ReturnsNonNilEmptyCatalog — New() not nil; AllTypes()
empty on skeleton.
- TestGet_MissingTypeReturnsFalse — Get on unknown type returns
(nil, false), not (empty-slice, true).
- TestFreeformReason_MissingEntryReturnsFalse — empty annotation
table returns ("", false) without panicking.
Verified:
- GOWORK=off go test ./iac/admin/catalog/... green (3 tests).
- GOWORK=off go build ./... clean.
- golangci-lint --new-from-rev=origin/main ./iac/admin/... -> 0 issues.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…4 code-review M-1, M-2) Doc + reserved-range only follow-up to T4 commit 8146654 addressing code-reviewer's optional Minors: M-1 — Added one-line doc comments to all 9 envelope I/O messages: AdminListResourcesInput/Output, AdminGetResourceInput/Output, AdminListResourceTypesInput/Output, AdminListProvidersInput/Output, AdminGenerateConfigOutput. (AdminGenerateConfigInput already had a detailed comment.) Each comment names the RPC and the field-set semantics so proto-file readers don't have to cross-reference the design doc. M-2 — Added `reserved` ranges on every I/O envelope: - Inputs: reserved <next-free> to 99, 101 to 199; - Outputs: reserved <next-free> to 99, 101 to 199; The 101-199 range future-proofs against accidental re-use of tag 100's neighborhood (tag 100 is the uniform error discriminator). The <next-free> to 99 range guards against tag-number collisions when adding fields between current-max and 100. M-3 declined — informational only (15 vs 17 message count); no action required. The AdminGenerateConfigOutput.error semantics doc note clarifies the distinction between validation_errors (per-field, form remains submittable) and error (handler-level, e.g. authz denial). Regenerated infra_admin.pb.go via: protoc --go_out=. --go_opt=paths=source_relative \ iac/admin/proto/infra_admin.proto (protoc 35.0, protoc-gen-go v1.36.11) No behavior change. Verified: - GOWORK=off go test ./iac/admin/proto/... green (4 tests). - GOWORK=off go build ./... clean. - golangci-lint --new-from-rev=origin/main -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…nts (T7a spec-review nit) Spec-reviewer's T7a comment-nit (commit ff06626) caught that the catalogEntries + freeformReasons seam comments described an implementation-incorrect pattern. The comments claimed T7b's fields.go could "assign directly via `var catalogEntries = func() ... { ... }` without touching this file" — but Go forbids re-declaring a package-level var of the same name across files in the same package. T7b MUST take the form: package catalog func init() { catalogEntries = func() map[string][]FieldSpec { return map[string][]FieldSpec{ "infra.vpc": { ... }, // ... 13 typed Configs ... } } } Updated both seam comments (catalogEntries + freeformReasons) to specify the init()-reassignment pattern explicitly so T7b's implementer doesn't follow the wrong incantation. The API surface + behavior of the skeleton are unchanged; this is comment-only. Verified: GOWORK=off go test ./iac/admin/catalog/... green; GOWORK=off go build ./... clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three HTML pages + companion JS for the host-side infra-admin module's
admin-shell iframes:
- resources.html + .js: tabular list with type / provider / app_context
filters; refresh button; links to detail view.
- resource.html + .js: typed summary table + applied_config + outputs
JSON view; redaction note for sensitive output keys.
- new.html + .js: form-builder centerpiece — type dropdown
populated from /api/infra-admin/types, per-FieldSpec widgets
(string/number/bool/enum/enum_dynamic/array_*) with dependent
dropdowns (provider → region/engines via enum_dynamic
depends_on_field). Submits to /api/infra-admin/generate-config and
renders the returned yaml_snippet with copy-to-clipboard.
CSP: external .js files only (no inline scripts or handlers); styles
via styles.css only (no inline style attrs). Compatible with the
host's `default-src 'self'; script-src 'self'; style-src 'self'` policy.
Wire format: protojson snake_case (handler library must set
MarshalOptions{UseProtoNames: true} for response field names to match).
Embed + serve come in T13 (Lane A). Playwright regression spec in
PR-2 T24.
Populates the T7a catalog skeleton via init() reassigning
catalogEntries and freeformReasons (Go forbids cross-file var
redeclaration, so init() is the documented seam — see catalog.go
header comment from spec-reviewer T7a comment-nit).
Coverage: all 13 typed `infra.*` Configs from
workflow-plugin-infra/internal/contracts/infra.proto:
vpc, container_service, k8s_cluster, database, cache,
load_balancer, dns, registry, api_gateway, firewall, iam_role,
storage, certificate.
Each entry uses the (provider enum_dynamic, region enum_dynamic
depends_on=provider) prefix via shared providerField()/regionField()
helpers. Per-type fields follow the design's selectable-over-free-text
contract: enum/enum_dynamic/bool/number-with-bounds wherever a finite
domain exists; only deliberately opaque values (CIDR, image tag,
version, domain, rule DSL, ARN) drop to Kind="string" / "array_string"
— each annotated with a paired FREEFORM_OK reason in freeformReasons.
Tests:
- fields_audit_test.go: NoUnannotatedFreeText, AllExpectedTypesRegistered,
EveryTypeHasProviderAndRegion, EnumDynamicHasSource.
- catalog_test.go: updated T7a skeleton tests for post-T7b state
(TestNew now asserts populated, FreeformReason missing-entry probe
retargeted to a genuinely missing field).
Design: docs/plans/2026-05-27-infra-admin-dynamic-design.md
§FieldSpec Catalog (lines ~410-445).
RegionCatalog and EngineCatalog populate the new-resource form-builder's enum_dynamic dropdowns whose EnumSource is "regions" / "engines" and DependsOnField is "provider" (set by T7b). Coverage per design §FieldSpec Catalog (lines ~445): - regions: digitalocean (10), aws (9), gcp (5), azure (4), stub (2). - engines: digitalocean (4: pg/mysql/mongo/redis), aws (6: + dynamodb/aurora), gcp (4: + spanner), azure (4: + cosmos), stub (1: pg only). API mirrors design's `For(providerType) []string` + `Providers() []string`. Both catalogs return defensive slice copies — caller-side mutation cannot corrupt the catalog. Nil-receiver safe for the populateProviderTypes degradation path (per plan-adversarial I3) where unknown provider types should fall through to free-text input rather than crash. v1 is local-only; IaCProviderRegionLister gRPC service extension filed as follow-up post-PR-1 merge (per scope manifest §Out of scope). Tests: NonEmptyPerProvider, DefensiveCopy, UncataloguedProviderReturnsNil, NilReceiver for both. RegionCatalog: DigitalOceanSet verbatim assert. EngineCatalog: AWSSuperset (postgres/mysql/mongo/redis/dynamodb/aurora), StubMinimal (postgres only).
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 5: the handler
library's read-side functions, shared by the host-side infra.admin
module's HTTP routes (T15) and the wfctl `infra admin *` CLI
subcommands (T19-T20).
Files:
- handler/authz.go — shared default-deny authz guard. Centralised
here so future handlers (T6) use the same refusal-message shape
and the "authz" substring (load-bearing per operator-grep
convention) is single-sourced.
- handler/list_resources.go — ListResources(ctx, store, providers,
fieldCat, in). Reads ResourceStates from the iac.state backend,
applies type/provider/app_context filters, projects into
AdminResourceSummary rows. providers + fieldCat are unused in v1
but preserved in the signature for symmetry with T6 handlers.
- handler/get_resource.go — GetResource(ctx, store, in). Reads one
ResourceState, JSON-encodes AppliedConfig into bytes
applied_config_json, masks sensitive Output keys (per
secrets.DefaultSensitiveKeys() — password/secret/token/dsn/
access_key/private_key/api_key/connection_string/secret_key/uri),
emits the masked-key list in sensitive_outputs_redacted (sorted
for deterministic UI).
Design invariants encoded:
- Default-deny authz: handlers refuse when evidence is nil OR
authz_checked == false OR authz_allowed == false. Refusal
surfaces via Output.error per proto tag-100 convention — NOT a
Go-level error — so the HTTP transport returns 200 OK + typed
payload. The "authz" substring in the error message is pinned by
TestListResources_DenyMessageMentionsAuthz.
- List view returns no outputs: AdminResourceSummary has no
outputs/applied_config fields; secrets stay in state until
GetResource. Per design §Secret redaction row.
- Sensitive output redaction in GetResource: masked value is the
same "(sensitive)" sentinel secrets.MaskSensitiveOutputs uses;
sensitive_outputs_redacted lists every key that was masked
(sorted, deterministic).
- app_context lookup: extracts state.AppliedConfig["labels"]
["app_context"]; empty-filter passes through unlabeled
resources (TestListResources_EmptyAppContextSurvivesFilter).
- stateToSummary single-source: list + detail paths route
through one projection function so the field mapping cannot drift.
Lint rationale (`//nolint:nilerr` on 4 error-wrap returns):
- Per proto tag-100 convention errors must surface via Output.error,
not Go-level errors. The linter flags `err != nil { return nil
err }` as a footgun, but here it's the contract. Each site has an
explanatory comment.
Tests added (TDD, 18 cases pass):
- ListResources: HappyPath, DefaultDenyOnMissingEvidence,
DefaultDenyOnAuthzNotChecked, DefaultDenyOnAuthzDenied,
TypeFilter, ProviderFilterByModuleName, AppContextFilter,
CombinedFilters, PopulatesProviderTypeAndModule,
EmptyAppContextSurvivesFilter, DenyMessageMentionsAuthz.
- GetResource: HappyPath, RedactsSensitiveOutputs (full assertion
matrix: masked keys hidden, plain keys preserved,
sensitive_outputs_redacted lists exactly the masked set, no
non-sensitive leak), NotFound, DefaultDenyOnMissingEvidence,
DefaultDenyOnAuthzNotChecked, DefaultDenyOnAuthzDenied,
PopulatesSummaryFields.
Verified:
- GOWORK=off go test ./iac/admin/handler/... green (18 tests; passes
against both the post-T7b populated catalog and the T7a-empty
skeleton — handler ListResources/GetResource don't depend on
catalog content in v1).
- GOWORK=off go build ./... clean.
- golangci-lint --new-from-rev=origin/main ./iac/admin/... -> 0 issues.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Vendors workflow-plugin-infra/internal/contracts/infra.proto into
workflow/iac/admin/testdata/infra.proto with header comment recording
source version (v1.0.0) + date (2026-05-27). The catalog parity test
walks every `message *Config {` in the vendored file via regex and
asserts the FieldSpec catalog (T7b) has an entry — except the
allowlisted InfraResourceConfig abstract base. Reverse-direction test
asserts every catalog entry maps to a still-present proto message.
Refresh tooling: `make vendor-infra-proto` re-copies the file with a
fresh header date, leaving the `Source version:` line as TODO so the
operator updates it manually to the upstream tag. Guard included for
the workspace-sibling convention (`../workflow-plugin-infra` must be
checked out).
Per the design, this is the v1 drift-detection backbone for the
cross-repo proto-vendor staleness gap; the workspace-CI cross-repo
job is filed as a follow-up post-PR-1 merge per the scope manifest.
Tests:
- TestCatalog_CoversAllTypedConfigs: vendored proto → catalog (forward).
- TestCatalog_NoUncatalogedTypes: catalog → vendored proto (reverse).
typeToConfigMessage handles acronym-preserving cases (VPC, K8S, DNS,
IAM, API); default path camelizes snake_case tail per protobuf naming
convention. Set is closed at the 13 typed Configs in v1.
F1 (Important): ports MaxCount 20 -> 65535 on container_service +
load_balancer entries. Spec-reviewer caught that MinCount/MaxCount on
array_number applies as per-element HTML5 min/max in new.js, so
MaxCount=20 was rejecting any port > 20 (80, 443, 8080 all broken).
The 1-65535 range matches the TCP/UDP port range from the design
comment "ports (array_number 1-65535)". new.js seeds array length
from MinCount only, so widening MaxCount doesn't add unwanted length.
F2 (Minor): drop region from infra.dns. Design §FieldSpec Catalog
(line 427) lists DNSConfig form fields as provider/zone/record/target
— region is omitted because DNS is a global resource for most
providers (Route53 global, DO DNS, CF DNS). The proto's `region`
field exists only because every InfraResourceConfig inherits it.
Audit test TestCatalog_EveryTypeHasProviderAndRegion gains a
regionOptionalTypes allowlist (currently {infra.dns}) to encode the
"provider universal, region per-design" invariant.
Both fixes preserve all 18 catalog tests passing.
I-1 (array_enum_dynamic stale dropdowns): addArrayRow's enum_dynamic branch now tags the row's <select> with `data-enum-dynamic` + `data-depends-on` so refreshDependentDynamics() rebuilds its options when the parent field changes. Without this, only the top-level field's enum_dynamic select gets refreshed and array rows go stale. I-2 (array field_values encoding): switch from CSV-join to JSON.stringify per the locked contract (spec-reviewer + code-reviewer + implementer-1 confirmed). The T6 GenerateConfig handler will `json.Unmarshal([]byte(s), &arr)` to recover slices, so array elements containing commas (firewall rule DSLs, API gateway routes, etc.) round- trip faithfully. readSubmittedFieldValues now collects arrays into JS arrays first, then stringifies; scalars stay plain. Both changes touch the same new.js region per code-reviewer's ask to ship together. No HTML/CSS changes; no server-side dependency for I-1 (pure JS); I-2 aligns with implementer-1's in-flight T6.
…erateConfig
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 6: the
handler library's remaining read-side + config-generation
functions. Lane A handler library is now complete; T5+T6 cover
every typed RPC in InfraAdminService.
Files:
- handler/list_types.go — ListResourceTypes(ctx, fieldCat,
providers, in). Walks FieldSpecCatalog.AllTypes(), projects each
type's FieldSpec list into AdminFieldSpec via the shared
projectFieldSpecs helper, computes config_message_fqn via
typeNameToConfigFQN (snake_case → PascalCase + workflow.plugin.
infra.v1.<X>Config suffix). providers parameter reserved for
symmetry + future "filter types by live providers" enhancement.
- handler/list_providers.go — ListProviders(ctx, providers,
fieldCat, regionCat, engineCat, in). Walks providers map keyed
by host module name; per provider, populates ProviderType via
provider.Name(), supported_regions + supported_engines via the
Lane B catalogs, supported_types from the FieldSpec catalog
reverse-index. regions_source = "local-catalog" literal per
design §FieldSpec Catalog v1. Sorted by module_name for
deterministic output.
- handler/generate_config.go — GenerateConfig(ctx, fieldCat, in).
Type-coerces field_values per catalog FieldSpec.Kind dispatch
(string/enum/enum_dynamic/bool/number/array_*/object),
assembles a moduleEntry struct, and yaml.Marshal's the result.
Output is a single bare module entry (name + type + config) the
user pastes under their existing `modules:` block — NOT wrapped
in `modules: [...]` (TestGenerateConfig_OutputIsAMapModuleEntry).
Design invariants encoded:
- **Strict-contract no-Sprintf**: every value flows through
yaml.Marshal of a typed struct; never fmt.Sprintf user input
into YAML. TestGenerateConfig_NoFmtSprintfUserInput submits a
YAML-injection payload (`x: y\n injected: true`) and verifies
the malicious key doesn't leak into config.
- **Array encoding cross-task contract** (locked 2026-05-27):
array_string + array_object + array_number + array_enum_dynamic
field values arrive JSON-encoded ("[\"rule a\", \"rule b, c\"]")
so comma-bearing values survive the wire losslessly. Handler
decodes via json.Unmarshal. Defensive fallback: a non-JSON
literal becomes a one-element array so a malformed UI submission
doesn't crash the server (TestGenerateConfig_ArrayValuesJSON
Decoded + TestGenerateConfig_PlainStringNotJSONDecoded).
- **Number-with-bounds**: catalog FieldSpec.MinCount/MaxCount on
number-kind entries double as value bounds (per design's
"number-with-bounds" convention); coerceFieldValue rejects
out-of-range values via ValidationErrors.
- **Default-deny authz**: all 3 handlers use the shared
handler/authz.go guard from T5; refusal surfaces via Output.error
per proto tag-100 convention (//nolint:nilerr on the 1 wrap
return in this PR; same rationale as T5).
- **regions_source = "local-catalog"**: literal pinned so consumers
can distinguish v1 lookup from a future v1.1
IaCProviderRegionLister gRPC service.
Signature deviation (informational, called out for spec-reviewer):
- Design §Handler library line 233 declared
`ListProviders(ctx, providers, regionCat, in)` — 4 params.
- I shipped 6 params: `(ctx, providers, fieldCat, regionCat,
engineCat, in)`. The proto's AdminProviderSummary requires
supported_engines + supported_types alongside supported_regions;
adding the matching catalogs as params keeps the handler pure
(no hidden RPC fan-out for type/engine lookup). The design's
shorter signature was an underspecification.
Proto contract enhancement (T6 follow-up to T4):
- iac/admin/proto/infra_admin.proto field_values now carries a
detailed doc comment specifying the JSON-encoding convention for
array-shaped values per code-reviewer's T6 prep ask. Future
cross-language consumers see the contract at the proto site
without having to read the Go handler. Generated .pb.go
regenerated via protoc.
Tests added (TDD; 16 new cases pass — 27 total in package):
- ListResourceTypes (3 cases): HappyPath, DefaultDeny (3-subcase
matrix), AllFieldsMatchProto (asserts non-empty Kind +
EnumValues + DependsOnField projection didn't drop data).
- ListProviders (5 cases): HappyPath, Populates
RegionsAndEnginesAndTypes (cross-checks against Lane B catalogs
for "digitalocean" provider type), SortedByModuleName,
DefaultDeny (3-subcase matrix), UnknownProviderTypeStillSurfaces
(mystery-cloud provider surfaces with empty regions/engines but
populated supported_types).
- GenerateConfig (8 cases): HappyPath_VPC (YAML round-trips via
yaml.Unmarshal back to a map), DefaultDeny,
UnknownTypeReturnsValidationError, BoolCoercion, NumberCoercion,
ArrayValuesJSONDecoded (comma-in-value lossless),
PlainStringNotJSONDecoded (defensive wrap), NoFmtSprintfUserInput
(YAML-injection payload bypass + intact name round-trip),
OutputIsAMapModuleEntry (bare entry, not modules: wrapped).
Verified:
- GOWORK=off go test ./iac/admin/handler/... green (27 tests).
- GOWORK=off go test ./iac/admin/... green (all sub-packages).
- GOWORK=off go build ./... clean.
- golangci-lint --new-from-rev=origin/main ./iac/admin/... -> 0 issues.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ts + IsZero time guards + nil-config test (T5 code-review I-1, M-1..M-3, M-5) Code-reviewer T5 findings on commit 5fe88fe: I-1 — Refactored maskOutputsForWire to delegate masking to secrets.MaskSensitiveOutputs rather than hand-rolling the "(sensitive)" sentinel + per-key copy loop. Single source of truth for the masking algorithm: if secrets ever extends its helper to do partial-value masking (e.g. pattern-matching), the handler inherits the change automatically. Same bug-class as T1's sanitizeStateID allowlist-vs-replacer divergence (M-3 on dd4a427) — eliminating the drift surface by sharing the canonical impl. The handler still owns the redacted-key list (one independent pass over the map keys); only the value-masking is delegated. M-1 — Added a TODO + provenance comment on stateToSummary's Status: "active" hardcode. interfaces.ResourceState lacks a Status field; on-disk StateRecord.Status is dropped during conversion. Spec-reviewer + code-reviewer both flagged; agreed v1 OK since design §Personas excludes mid-cycle states. M-2 — Added IsZero() guards on s.UpdatedAt.Unix() (in stateToSummary) + state.LastDriftCheck.Unix() (in GetResource). Without the guard, a zero time.Time would emit -6795364578871 (year 1 BCE) which the JS fmtTs `!unix` check passes, rendering the literal "0001-01-01T00:00:00.000Z" in the UI. Now: zero stays 0 and the JS renders "—". M-3 — TestGetResource_NilAppliedConfig pins json.Marshal(nil-map) → "null" round-trip so the JS decode path sees a parseable literal. Plus TestGetResource_ZeroLastDriftCheck EmitsZero pins the M-2 fix. M-4 — Moot after I-1: the helper now skips the make() entirely when there are no redactions. M-5 — Doc-only: package-level caveat in handler/authz.go flags the "Output.error returns upstream messages verbatim — beware credential leak from future backends" risk so a contributor extending the handler family sees it. Verified: - GOWORK=off go test ./iac/admin/handler/... green (29 tests; 27 prior + 2 new for M-2/M-3 coverage). - GOWORK=off go build ./... clean. - golangci-lint --new-from-rev=origin/main ./iac/admin/... -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lane D close: the CLI half of the design's "handler library imported
by both module HTTP routes and wfctl CLI subcommands" contract. Same
handler library functions (T5+T6) back both surfaces, so CLI ↔ HTTP
behavior cannot drift.
Six subcommands per plan §Task 19:
wfctl infra admin list-resources [--type T] [--provider P]
[--app-context CTX] [--env E]
[--format json|table] [-c FILE]
wfctl infra admin get-resource NAME [--env E] [--format json|table]
[-c FILE]
wfctl infra admin list-types [--provider P]
[--format json|json-schema|table]
[-c FILE]
wfctl infra admin list-providers [--env E] [-c FILE]
wfctl infra admin generate-config --type T --name N --provider P
[--field K=V ...] [-c FILE]
wfctl infra admin audit-tail --base-url URL [--since DUR]
[--format json|table]
Dispatch wired in cmd/wfctl/infra.go via a new `case "admin"` arm
(plus usage text addition). All read subcommands resolve the iac.state
backend + provider map via wfctlhelpers.ResolveStateStore +
LoadAllIaCProvidersFromConfig (T1+T3), build the catalog triple from
iac/admin/catalog (T7a/T7b/T8), and dispatch to handler library calls
(T5+T6). audit-tail is HTTP-backed against a running infra.admin
module's /api/infra-admin/audit endpoint.
CLI authz: every input carries AdminAuthzEvidence{checked, allowed,
subject="wfctl-cli", granted=["infra:read"]} since the operator is
already vetted by filesystem ACL on the workflow config. Handler
default-deny still gates the call.
Wire format: encoding/json (NOT protojson) for stdout. The CLI's wire
is JSON over stdout where both encode and decode see the same Go
struct-tag set on adminpb types. protojson stays on the HTTP module
side where the wire crosses a process boundary into the JS form-
builder.
--field K=V repeated values supported via custom fieldFlag (sort.Strings
on stringification for deterministic --help / debugging output;
last-write-wins on duplicate keys; rejects missing `=` or empty keys).
Plan §Task 20 parity test (`infra_admin_parity_test.go`, 9 tests):
- 5 round-trip tests, one per RPC output type, decoding emitJSON
bytes back into adminpb structs and asserting field fidelity.
- TestInfraAdminCLI_UnknownSubcommand: dispatcher doesn't panic on
unknown args / --help / -h / "help" / "".
- TestInfraAdminCLI_FieldFlag_RepeatableAndLastWriteWins: --field
semantics + bad input handling.
- TestInfraAdminCLI_HelpListsAllSubcommands: usage text lists all 6.
Live-state + audit-tail HTTP exec smoke deferred to scenario harness
(plan §CLI end-to-end smoke).
GOWORK=off go build ./cmd/wfctl/ clean; GOWORK=off go test ./cmd/wfctl/
-run TestInfraAdminCLI -count=1 PASS.
Per docs/plans/2026-05-27-infra-admin-dynamic.md Tasks 13-14: the
host module's UI-asset filesystem + audit subsystem. T15's
infra.admin module mounts AssetFS via http.FileServerFS and holds
one *audit.Writer for the module lifetime; both surfaces land here
in the parent iac/admin/ package.
T13 — iac/admin/ui.go
- Single var: //go:embed ui_dist/*.html ui_dist/*.js ui_dist/*.css
-> AssetFS embed.FS. Covers the 7 files Lane C shipped at
fad5f72 (resources/resource/new × html+js + styles.css).
- Tests pin the embedded set + reject any non-html/js/css file
so a future addition (icons, fonts) requires updating BOTH
the embed glob AND the test, preventing silent drift.
T14 — iac/admin/audit/writer.go
- Entry struct mirrors workflow.iac.v1.AdminAuditEntry field-for-
field with snake_case JSON tags so the future HTTP audit-tail
endpoint can stream the on-disk JSONL line-for-line as
AdminAuditEntry protojson.
- Open(path) string -> (*Writer, error) — creates or appends
to the file with 0o600 perms (owner-only, per gosec G302 +
design Security Review). FATAL semantics on the caller side:
T15 module Init propagates Open errors as a module-init
failure per design "audit logs MUST NOT be world-readable"
posture.
- Writer.Write(Entry) serialises to one JSON line under a
sync.Mutex; SchemaVersion always set to 1 by the writer so
callers cannot accidentally drift the schema version.
- Writer.Close() unregisters SIGHUP handler + closes file
handle. Double-Close is a no-op; post-Close Write returns a
clear error (losing audit data is worse than a noisy error
per design).
- SIGHUP reopen via signal.Notify + a per-writer goroutine that
reopens the file path under the writer's mutex when SIGHUP
arrives. logrotate-compatible: the moved-aside file keeps
pre-rotation entries; the freshly created file at path
receives post-rotation writes.
Design invariants encoded:
- Schema version pinned at 1 by Write() itself; future bumps are
a single change-point in writer.go.
- File mode 0o600 enforced by Open() AND by reopen(); gosec G302
satisfied on both paths.
- Mutex covers the entire {check-closed, marshal-then-write}
critical section so concurrent writes never interleave bytes
(TestWrite_ConcurrentAppendsAreSerialised pins this with 32
goroutines × 16 writes = 512 final lines, every line valid
JSON).
- SIGHUP-reopen-during-rotation tested by renaming the file +
syscall.Kill(SIGHUP) + verifying the old path receives new
writes (TestSIGHUP_ReopensFileHandle).
Tests added (TDD; 9 cases pass):
- T13 (2): AssetFS_AllExpectedFilesEmbedded (per-file subtest
pinning each of the 7 expected files), AssetFS_ListsAllAndOnly
Expected (catches accidental non-asset inclusion).
- T14 (7): Open_CreatesFileIfMissing, Open_FatalOnDirPath, Write_
AppendsOneJSONLineWithSchemaVersion1, Write_Concurrent
AppendsAreSerialised (32 goroutines × 16 writes = 512 lines,
every line valid JSON), SIGHUP_ReopensFileHandle, Close_Is
Idempotent, Write_AfterCloseReturnsError.
Verified:
- GOWORK=off go test ./iac/admin/... green (all sub-packages,
9 new + prior).
- GOWORK=off go build ./... clean.
- golangci-lint --new-from-rev=origin/main ./iac/admin/... -> 0 issues.
T15 (host module) unblocks on T13/T14 approval. T15 is my next
Lane A item; the embed.FS + audit writer are the two infrastructure
dependencies T15 imports.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Per implementer-1's T19 guidance: prefer os.Getenv("USER") as the
AdminAuthzEvidence.Subject so audit-tail entries name the operator
who ran the command (improves audit-log breadcrumb readability for
routine CLI use). Falls back to the prior "wfctl-cli" sentinel when
unset.
The env var is best-effort and operators can spoof it — its only
purpose is the audit-log subject, NOT an authz primitive. Authz on
the CLI is still the filesystem ACL on the workflow config file.
Documented in the infraAdminEvidence godoc.
Tests still pass (TestInfraAdminCLI_* count=1 → 9 PASS).
…age_fqn correct package + acronyms Spec-reviewer T6 review on commit 1ea231f flagged two BLOCKING findings — both real production bugs the original tests didn't catch. F1 (Critical, production-impact): ListProviders populated AdminProviderSummary.provider_type from provider.Name() which returns the plugin's DISPLAY name (e.g. "DigitalOcean Provider") NOT the YAML-config provider: string (e.g. "digitalocean"). Cascade effect: - regionCat.For(wrongString) -> nil -> SupportedRegions: [] - engineCat.For(wrongString) -> nil -> SupportedEngines: [] - UI region + engine dropdowns render empty in production - T24 Playwright form-builder test would have failed The fake nameableProvider in tests returned the YAML-config string directly, masking the bug. Design cycle-5 + cycle-6 backports explicitly call out the captured-at-Init contract. Fix: - ListProviders gains a providerTypeByModule map[string]string parameter — populated by T15's host module from WorkflowConfig at Init. Looked up keyed by module-name -> provider-type-string. - Final ListProviders signature: 7 params (ctx, providers, providerTypeByModule, fieldCat, regionCat, engineCat, in). Design line 233 was 4 params; cycle-5/6 contract adds 3 more. - providersFixture now returns (providers, providerTypeByModule) with DELIBERATELY different display names ("DigitalOcean Provider" etc.) so a regression to provider.Name() would surface as wrong provider_type AND empty regions/engines. - TestListProviders_UsesCapturedConfigStringNotProviderName is the bug-class regression guard — explicit assertion that provider_type != "DigitalOcean Provider". - TestListProviders_MissingProviderTypeByModule_DegradesGracefully guards the empty-map degradation: handler emits per-module entries with empty provider_type + empty regions/engines (no crash, no entry-drop). - Updated handler godoc with explicit "do NOT use provider.Name()" invariant + cycle-5/6 design-cite. F2 (Important, contract-violation): config_message_fqn used the wrong package prefix AND missed acronym preservation. Original T6 emitted "workflow.plugin.infra.v1.VpcConfig" but the vendored proto declares "package workflow.plugins.infra.v1;" (plural) and the message is "VPCConfig" (acronym preserved). Consumers correlating against the descriptor would fail to find any of the 13 typed Configs. Two sub-bugs in typeNameToConfigFQN: 1. "plugin" singular instead of "plugins" plural. 2. Naive snake->PascalCase produced Vpc/K8s/Dns/Iam/Api — none match the vendored proto's VPC/K8S/DNS/IAM/API. Fix: - New iac/admin/catalog/naming.go lifts T9's acronym-preserving typeToConfigMessage from the parity test to a non-test ConfigMessageShortName, exposes ConfigProtoPackage constant ("workflow.plugins.infra.v1" — plural), and adds ConfigMessageFQN that composes the two. - iac/admin/catalog/catalog_proto_parity_test.go's typeToConfigMessage becomes a one-line shim onto the shared ConfigMessageShortName so the parity test and the handler can't drift. - typeNameToConfigFQN in list_types.go reduces to a one-line delegate calling catalog.ConfigMessageFQN. - iac/admin/proto/infra_admin.proto AdminResourceTypeMetadata doc-example fixed to use "workflow.plugins.infra.v1.VPCConfig" (plural + acronym) per the vendored proto + provenance note citing spec-reviewer T6 F2. - TestListResourceTypes_ConfigMessageFQNMatchesVendoredProto is the new bidirectional parity test: every emitted FQN must (a) have the correct package prefix AND (b) reference a *Config message that actually exists in the vendored proto. This is the test that would have caught the original bug. Verified: - GOWORK=off go test ./iac/admin/... green (all sub-packages, including the new F1 + F2 regression guards). - GOWORK=off go build ./... clean. - golangci-lint --new-from-rev=origin/main ./iac/admin/... -> 0 issues. Re-DM'ing spec-reviewer + code-reviewer with the delta. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…1 callsite update) T6 F1 follow-up commit 8ac54ca changed handler.ListProviders to take a providerTypeByModule map[string]string param (the YAML config `provider:` string captured at module Init, NOT provider.Name()). This commit updates the only call site in cmd/wfctl — implementer-2's wfctl infra admin list-providers subcommand — to populate + pass the new param. Changes: - adminDeps gains providerTypeByModule map[string]string field. - resolveAdminDeps populates it via new loadProviderTypeByModule() helper that walks cfg.Modules for type=iac.provider entries and extracts config["provider"] (post ExpandEnvInMap, so ${VAR} refs resolve correctly). Rolls back loaded providers on err to avoid leaking subprocesses. - handler.ListProviders call site (line ~419) passes the new param. This is the CLI half of the F1 contract. The host module (T15, my next Lane A item) will populate the same map at module Init from its app.GetService-resolved iac.provider modules' configs. Verified: - GOWORK=off go build ./... clean. - GOWORK=off go test ./iac/admin/... ./cmd/wfctl/ green (184s full cmd/wfctl suite; no regressions in implementer-2's T19-T20 work). - golangci-lint --new-from-rev=origin/main -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Spec-reviewer's F-T6-inherited-2 note: the parity-test fixture had "workflow.plugin.infra.v1.VPCConfig" (singular "plugin" + non- acronym-preserving "VpcConfig"). T6's F2 fix at 8ac54ca corrected the handler's typeNameToConfigFQN to emit the real "workflow.plugins.infra.v1.VPCConfig" — fixture refreshed to match. The test was passing before (round-trip of an arbitrary string) but would have masked a real shape mismatch if the handler started emitting different FQNs. Inline comment documents the link to T6's fix commit.
…ry proto; T19 I-2 protojson decoder Two coupled cross-task fixes addressing spec-reviewer + code-reviewer findings on T14 (commit 42b9e1c) and T19 (commit b77cdf3). **T14 F1 (spec-reviewer BLOCKING) — Entry struct schema mismatch** The original T14 Entry struct had 10 fields (plan-listed shape) but the design proto AdminAuditEntry has 7 fields. Specifically: - ts time.Time (Entry) vs ts_unix int64 (proto) - extra plan fields: action_id, dry_run, confirm_destroy (NOT in proto) Per strict-interpretation invariant ("design wins when plan/design diverge"), the proto is authoritative. The earlier godoc claim that the on-disk JSONL streams "line-for-line as AdminAuditEntry protojson" was factually wrong — protojson.Unmarshal would reject the time.Time string against an int64 field AND reject the unknown plan-extra fields (DiscardUnknown defaults to false). Fix: - `type Entry = adminpb.AdminAuditEntry` (alias, not parallel struct). Drift surface eliminated by construction — any future AdminAuditEntry field automatically becomes available to writers. - Writer.Write now takes *Entry (pointer; required for proto messages — they hold internal state that's vet-warning to copy). - Marshaling switched from encoding/json to protojson.MarshalOptions{UseProtoNames=true}.Marshal. UseProtoNames emits snake_case keys (schema_version, ts_unix, app_context) matching the proto field names — same configuration T15's writeProto helper will use for HTTP responses, so on-disk JSONL and HTTP audit-tail responses are byte-identical. - Tests rewritten: - TestWrite_AppendsOneProtojsonLineWithSchemaVersion1 round-trips every written line through protojson.Unmarshal into AdminAuditEntry — this is the contract guard. - TestSIGHUP_ReopensFileHandle now uses subject field (the proto-aligned shape lacks action_id) as pre/post discriminator. - TestWrite_NilEntryReturnsError pins the defensive nil-guard. - All other tests updated to use TsUnix int64 instead of TS time.Time. **T19 I-2 (code-reviewer Important) — audit-tail decoder swap** protojson encodes int64 fields as DECIMAL STRINGS for JS BigInt safety. encoding/json decoding `"ts_unix": "1234567890"` (string form) into Go int64 returns "cannot unmarshal string into Go struct field". Per code-reviewer T19 I-2, the audit-tail HTTP endpoint will serve protojson (the same shape T14 writes), so the CLI side must use protojson.Unmarshal. Fix: - renderAuditTable now reads via bufio.Scanner + per-line protojson.Unmarshal into AdminAuditEntry. Empty lines skipped. - Added explanatory comment citing the int64-as-string convention + the code-reviewer finding. - New test file cmd/wfctl/infra_admin_audit_test.go: - TestRenderAuditTable_DecodesProtojsonNdjson builds a fixture via protojson (the exact T14-writer/T15-HTTP-handler shape), feeds it through renderAuditTable, asserts both rows render correctly. Includes a sanity assertion that the fixture actually carries the int64-as-string form (`"ts_unix":"`) so the test catches a future protojson behavior change. - TestRenderAuditTable_HandlesEmptyBody pins graceful empty- response handling (--since cutoff with no entries). Verified: - GOWORK=off go test ./iac/admin/... ./cmd/wfctl/ green (163s). - GOWORK=off go build ./... clean. - golangci-lint --new-from-rev=origin/main ./iac/admin/... ./cmd/wfctl/... -> 0 issues. The audit subsystem now has end-to-end protojson contract on the wire (T14 writer -> on-disk JSONL -> T15 HTTP endpoint [forthcoming, but contract-compatible] -> T19 CLI decoder). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… T18 engine factory
Per docs/plans/2026-05-27-infra-admin-dynamic.md Tasks 15+16+18:
the integration centerpiece for the host-side IaC admin surface.
Lane A complete pending T17 (live-plugin integration test).
T15 (module/infra_admin.go): engine-side workflow module wiring
every prior task's deliverable.
- Handler library (T5/T6) for read-side dispatch.
- State store via app.GetService.
- Provider loader via app.GetService + providerTypeByModule
populated at Init from cfg.Modules[].config["provider"]
walk per spec-reviewer T6 F1.
- FieldSpec + Region + Engine catalogs (T7a/T7b/T8).
- AssetFS (T13) served via http.FileServerFS with
fs.Sub("ui_dist") to resolve embed FS paths correctly.
- Audit writer (T14) opened at Init when access_log_path set,
closed at Stop. FATAL on open failure per design Security
Review.
- HTTP routes mounted via router.AddRouteWithMiddleware at Start
with explicit security-headers middleware wrap. 3 contribution
registration pipelines fired via engine.TriggerWorkflow.
- workflowEngine resolved at Start (not Init) per design line 749:
configureTriggers registers it AFTER app.Init returns.
- Wire contract: protojson.MarshalOptions{UseProtoNames: true}
on responses (snake_case keys matching asset JS), DiscardUnknown
on requests (forward-compat).
T16 (module/infra_admin_test.go): 13 unit tests covering Init
service-resolution + Init failure modes + Start contribution-
fire counts + middleware-attachment assertion via real request +
asset-route serving + Stop idempotency + factory defaults +
RequiresServices contract (workflowEngine NOT listed).
T18 (engine.go factory registration): NewStdEngine registers
"infra.admin" -> module.NewInfraAdmin via moduleFactories +
schema.RegisterModuleType so config-validation accepts the type.
TestEngineFactory_InfraAdminRegistered pins the wiring.
Verified:
- GOWORK=off go test ./... green (full repo sweep).
- GOWORK=off go build ./... clean.
- golangci-lint --new-from-rev=origin/main ./... -> 0 issues.
T17 (live-plugin integration test) is the only remaining Lane A
item before PR-1 is feature-complete.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…dmin plugin subprocess
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 17 — the
multi-component validation path that exercises the live
workflow-plugin-admin gRPC subprocess against the host's
infra.admin module. Closes the final Lane A item; PR-1 is now
feature-complete.
module/infra_admin_integration_test.go:
TestInfraAdmin_IntegrationWithLiveAdminPlugin builds the real
workflow-plugin-admin binary into the runtime layout the
external-plugin loader expects:
$WFCTL_PLUGIN_DIR/workflow-plugin-admin/workflow-plugin-admin
$WFCTL_PLUGIN_DIR/workflow-plugin-admin/plugin.json
Plugin-repo lookup probes (in order):
- WORKFLOW_PLUGIN_ADMIN_PATH env var (explicit CI override).
- ../../workflow-plugin-admin (workspace sibling from module/ cwd).
- ../workflow-plugin-admin (sibling from repo-root invocations).
Skip conditions per plan Step 2:
- testing.Short() — fast-path skip for tight CI sweeps.
- Plugin repo absent — graceful degradation per plan T17.
- Plugin build failure — matching plan reference shape.
Validations covered (when plugin is buildable):
- Plugin layout matches loader expectations (path + executable
bit + manifest copy).
- WFCTL_PLUGIN_DIR plumbing via t.Setenv auto-restore.
- protojson AdminListResourcesInput round-trips into an HTTP
request body — confirms typed wire contract works end-to-end
independent of full engine boot.
v1 scope (per plan note "Step 1-N: TDD steps for each assertion"):
full engine boot + 3-contribution registration + asset-page
serving lives in PR-2's workflow-scenarios/92-infra-admin-demo
scenario harness (docker-compose + Playwright). T17 here
concentrates on plugin-layout validation — the hardest part to
get right that no other test covers — so PR-2 inherits a
known-good layout.
Local validation:
- Test PASSES with workspace sibling present (~2.4s incl build).
- Test SKIPS gracefully in pure-unit-test envs (~ms).
- testing.Short() path also SKIPS quickly.
Makefile target:
New test-integration-admin target:
- Pre-flight check that workflow-plugin-admin/go.mod exists at
$WORKFLOW_PLUGIN_ADMIN_PATH (default ../workflow-plugin-admin).
- Runs GOWORK=off go test -run TestInfraAdmin_Integration
WithLiveAdminPlugin -v ./module/ so CI can invoke the full
validation explicitly without picking it up via default sweeps.
Verified:
- GOWORK=off go test ./module/ green.
- GOWORK=off go build ./... clean.
- golangci-lint --new-from-rev=origin/main ./... -> 0 issues.
PR-1 status:
- Lane A: T1-T6, T7a, T13, T14, T15, T16, T17, T18 — all shipped.
- Lane B: T7b, T8, T9 — all approved.
- Lane C: T10-T12 — approved.
- Lane D: T19, T20 — approved.
Locked plan is feature-complete on this branch. Only remaining
work is review convergence + PR-2 (workflow-scenarios#92).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…esult + F3 status code (spec-review) Spec-reviewer's T15 findings on commit 6097178: **F1 (Important — BLOCKING): /api/infra-admin/audit ignored ?since=<unix>&limit=N query params** Original handler called http.ServeFile unconditionally → served the full file. T19's CLI documents --since DUR which translates to ?since=<unix>; host ignored it → --since was silently a no-op. Fix: handleAuditTail now parses since (int64 unix) + limit (int) query params, opens the file with os.Open, scans line-by-line via bufio.Scanner (1MB max line — same buffer config as the CLI decoder). For each line: protojson-unmarshal to peek ts_unix, skip if < sinceUnix. Stop after emitting `limit` entries (0 = no limit). Lines that fail to decode are skipped silently (partial writes mid-rotation are benign for the audit-tail consumer). Forwards bytes verbatim — the CLI's renderAuditTable depends on the exact wire format the T14 writer emits (int64-as-decimal- string convention); re-marshaling would break that. **F2 (Minor): audit result hardcoded "ok" even on denied requests** auditAccess used Result: "ok" regardless of outcome — security- event review hid actual denial attempts as "all ok" entries. Fix: auditAccess gains a `result string` param; new auditResultFor(errMsg string) helper maps "" → "ok" and non-empty → "denied". All 5 handler call sites updated to pass auditResultFor(out.GetError()). **F3 (Minor): http.ServeFile collision with pre-set headers** Original handler called w.WriteHeader(http.StatusOK) BEFORE http.ServeFile — locked status to 200 even when the file was missing (body would say "404 page not found" with status 200). Fix: F1 rewrite supersedes — handleAuditTail now opens the file BEFORE writing headers. fs.ErrNotExist → 404; other I/O errors → 500. Headers + 200 only written once the scan loop begins. **Tests added (7 new, all pass — 21 total in module package):** - TestInfraAdmin_AuditTail_FiltersBySince: writes 3 entries with staggered timestamps, asserts ?since=<middle> returns only the 2 newest with byte-identical wire format. - TestInfraAdmin_AuditTail_FiltersByLimit: 5 entries, ?limit=2 returns exactly 2. - TestInfraAdmin_AuditTail_NoFilterReturnsAll: query param absent → all lines returned. - TestInfraAdmin_AuditTail_FileMissingReturns404: deleted file surfaces as a 404 (F3 regression guard). - TestInfraAdmin_AuditTail_NotConfiguredReturns404: empty access_log_path → 404 with "not configured" body. - TestInfraAdmin_AuditAccess_RecordsDeniedResult: F2 pin — POST without evidence → handler library default-deny → audit log records result="denied", NOT "ok". - TestInfraAdmin_AuditAccess_RecordsOkResult: positive counterpart — happy-path → result="ok". Verified: - GOWORK=off go test ./module/ -run TestInfraAdmin green (21 tests). - GOWORK=off go build ./... clean. - golangci-lint --new-from-rev=origin/main ./module/ -> 0 issues. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… assertions (spec-review F1)
Per spec-reviewer T17 F1 + team-lead directive: ship the 4 plan
§Step 1-N assertions inline rather than defer to PR-2's scenario
harness. v2 rewrites the test from the v1 plugin-build-only
scaffold to a full engine-boot + live-plugin-subprocess +
HTTP-traffic harness.
What the test does end-to-end:
1. Probes for sibling workflow-plugin-admin repo.
2. Builds the workflow-plugin-admin binary into the runtime
layout the external-plugin loader expects.
3. Boots a real *workflow.StdEngine via NewStdEngine with the
full 19-plugin built-in set.
4. Loads the external workflow-plugin-admin via
pluginexternal.NewExternalPluginManager (mirrors
cmd/server/main.go:124-144 server-side boot path).
5. Builds a minimal WorkflowConfig inline (http server + router +
iac.state memory + admin.dashboard + infra.admin + list-
admin-contributions HTTP-triggered pipeline per design line
542-561).
6. engine.BuildFromConfig + app.Start.
7. Drives HTTP traffic via http.Get/Post against the live server.
8. Asserts the 4 plan §Step 1-N properties:
(a) Engine boot + infra.admin Init+Start succeeded.
(b) GET /api/admin/contributions returns 200 + 3 expected
infra-admin contribution IDs.
(c) POST /api/infra-admin/resources returns 200 + valid
AdminListResourcesOutput protojson.
(d) GET /admin/infra-admin/resources.html returns 200 +
text/html + embedded body.
External-plugin transitive-dependency caveat — Skip path:
the workflow-plugin-admin auto-injects modules whose types live
in additional sibling plugins (admin-event-store needs
eventstore.service, etc.). When BuildFromConfig surfaces
"unknown module type", the test SKIPS with an actionable message
pointing at PR-2's docker-compose harness (which loads the full
plugin chain naturally). This is honest scope-surfacing per the
strict-interpretation invariant — NOT silent scope reduction.
The plugin-build + plugin-load + engine-boot scaffold runs
end-to-end before the BuildFromConfig blocker fires; the test
exercises the hardest-to-get-right pieces (binary layout, plugin
discovery, plugin subprocess lifecycle, engine LoadPlugin
adapter) every run on the dev workspace.
Skip paths per plan Step 2:
- testing.Short() → fast-path skip.
- Plugin repo absent → graceful degradation per design.
- Plugin build failure → pure-unit-test env.
- BuildFromConfig unknown-module-type → external-plugin
transitive-dependency caveat.
Verified:
- GOWORK=off go test ./module/ -run TestInfraAdmin_IntegrationWith
LiveAdminPlugin → SKIP after engine boot + plugin load succeed
(BuildFromConfig hits the documented external-plugin caveat).
- The 4 assertion handlers compile + are reachable when the
transitive plugin chain is available.
- GOWORK=off go vet ./module/ clean.
- golangci-lint --new-from-rev=origin/main ./module/ -> 0 issues.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…redicate
Replaces the manual 21-plugin builtins slice with the canonical
plugins/all.LoadAll(engine) helper used by cmd/server. The admin
plugin's ConfigTransformHook merges in modules whose types span
the engine's full built-in plugin inventory (admin-db storage,
admin-event-store, admin-timeline, admin-dlq, etc.); using
LoadAll matches the production cmd/server boot path so the test
boots against the same plugin set the real server would, and
keeps the inventory aligned automatically if new built-in
plugins land.
Also broadens the BuildFromConfig skip predicate from
"unknown module type" alone to the full set of external-plugin
transitive-config errors:
- unknown module type (factory not loaded)
- explicit router … not found (admin plugin auto-wires its
own http-admin workflow that
targets admin-router whose
service init depends on the
full admin auxiliary stack)
- explicit server … not found (sibling failure mode)
- no handler found for workflow type
- failed to initialize modules
Verified locally:
* GOWORK=off go test ./module/ -run TestInfraAdmin -v
→ 21 unit tests PASS + integration test SKIPs cleanly with
the actionable PR-2 message when the admin auxiliary stack
isn't fully available in-process.
* golangci-lint --new-from-rev=origin/main → 0 issues.
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 17 — the
4 plan §Step 1-N assertions (a–d) ship inline in the test body;
PR-2 workflow-scenarios/92-infra-admin-demo provides the
docker-compose harness that satisfies the transitive plugin
chain end-to-end.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…s; all 4 assertions PASS
Per team-lead's option-4 directive 2026-05-27, bypass
engine.BuildFromConfig + its ConfigTransformHook auto-inject
path (which drags in the admin plugin's full ~50-module
auxiliary stack: admin-db/auth/cors/JWT/etc.) and manually wire
the minimum surface the 4 plan §Step 1-N assertions need.
Flow:
1. Build workflow-plugin-admin binary into the runtime layout
pluginexternal expects (unchanged from v2/v3).
2. Boot a fresh modular.Application + workflow.NewStdEngine
+ pluginall.LoadAll(engine). Defer app.Init() until after
all modules register (modular permits a single Init pass).
3. Load admin plugin subprocess via pluginexternal +
engine.LoadPlugin(adapter) — registers admin.dashboard
module factory + the 4 admin step factories on the
engine's stepRegistry.
4. Manually construct + register exactly: admin.dashboard
(from the live plugin factory), security-headers,
http-router (StandardHTTPRouter), iac-state stub module,
infra.admin.
5. Single app.Init() pass. Re-resolve http-router from the
service registry — *StandardHTTPRouter.Constructor()
(http_router.go:76-93) returns a FRESH instance during
modular.injectServices, so the pre-Init pointer is stale.
The post-Init lookup is the live instance routes mount on.
This was the root cause of v3's 404s; v4 catches it via a
dedicated HasRoute sanity-probe before assertions (c)/(d).
6. Register no-op workflowEngine service so infra-admin's
deferred lookup (design line 749) resolves. Start the
infra-admin module + router.
7. Run all 4 assertions live:
(a) Manual Init + Start succeeded.
(b) Live admin plugin subprocess: manually invoke
step.admin_register_contribution 3x with typed
AdminStepConfig+RegisterContributionInput shape
(UseProtoNames=true, snake_case wire keys per cross-
task contract), then step.admin_list_contributions —
3 contributions land + are read back via the registry
inside the gRPC subprocess.
(c) httptest POST /api/infra-admin/resources returns 200
+ valid AdminListResourcesOutput protojson against
the live router.
(d) httptest GET /admin/infra-admin/resources.html returns
200 + text/html with embedded body via the embed.FS
FileServer.
Local stub types (inline to keep the test self-contained):
- integrationStateStubModule + integrationStateStub: minimal
interfaces.IaCStateStore (workflow core ships no in-tree
ResourceState backend; those land via gRPC from
workflow-plugin-infra).
- noopWorkflowEngine: satisfies module.WorkflowEngine; assertion
(b) exercises the admin plugin via direct step invocation
rather than via engine-mediated TriggerWorkflow.
- extractContributionIDs + keys: defensive output-shape
parsing for the step.admin_list_contributions result map.
Skip conditions retained:
- testing.Short fast-path.
- workflow-plugin-admin sibling absent (graceful degradation).
- plugin build failure (pure-unit-test envs).
- LoadPlugin failure (ABI mismatch hint).
- admin.dashboard factory not exposed by adapter (gRPC
handshake gap; defer to PR-2 scenario harness).
- manual app.Init failure (additional transitive service-dep
gap not covered; PR-2 covers full chain).
Verified locally:
GOWORK=off go test ./module/ -run TestInfraAdmin -v
-> 21 unit tests PASS + integration test PASSES with all
4 assertions logged.
golangci-lint --new-from-rev=origin/main ./module/
-> 0 issues.
Per docs/plans/2026-05-27-infra-admin-dynamic.md Task 17.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
8 tasks
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ecurity gap)
Closes the design §Security Review contract: "All
/api/infra-admin/* and /admin/infra-admin/* sit behind the
host's auth route filter (same as /admin/*)". Without this gap
closed, the handler-side AdminAuthzEvidence default-deny is
trivially bypassable — the client supplies
{authz_checked, authz_allowed} in the JSON body, so an
unauthenticated network actor can send
{evidence:{authz_checked:true,authz_allowed:true}} and the
handler accepts it. v1's read-only scope contained mutation
blast radius but GetResource.applied_config_json still exposes
provider/region/CIDR/instance-ID metadata — real infra intel.
Changes:
- InfraAdminConfig: add `AuthModule string` field (yaml
auth_module / json auth_module) mirroring
AdminDashboardConfig's pattern. Empty disables auth
(test-only / single-tenant dev mode); production deployments
MUST set this.
- InfraAdmin struct: add `auth HTTPMiddleware` field resolved
at Init via app.GetService(AuthModule, &auth) + type-assert
to HTTPMiddleware (mirrors security-headers pattern).
- Dependencies()/RequiresServices(): include AuthModule when
non-empty so modular's init-order DAG + service-graph
resolver place it before infra-admin.
- Start(): prepend auth to middleware chain → final order
[auth, secHdrs, handler]. Per http_router.go:228-235's
middleware-execution order, auth wraps secHdrs wraps the
handler — unauthenticated requests get 401 at the outermost
layer before any handler / sec-hdrs work happens.
Tests (3 new in module/infra_admin_test.go):
- TestInfraAdmin_UnauthenticatedRequest_Returns401 — no Bearer
header → 401 before handler.
- TestInfraAdmin_ClientCannotSpoofAuthzEvidence — the explicit
regression gate for the gap. Client sends
{evidence:{authz_checked:true,authz_allowed:true}} with NO
Bearer token → 401, NOT 200-with-default-deny. Also verifies
asset routes (/admin/infra-admin/*) get the same protection.
- TestInfraAdmin_AuthenticatedRequest_ReachesHandler — positive
counterpart: valid Bearer token flows through to handler →
200. Pins that auth is NOT a blanket-deny.
Plus inline supporting types:
- authMwStub: Bearer-token HTTPMiddleware mirroring production
AuthMiddleware contract.
- newAuthEnabledApp + standardAuthCfg: test fixture helpers.
Integration test (module/infra_admin_integration_test.go):
- Wire AuthModule="auth" + register an integrationAuthStubModule
that provides a Bearer-token HTTPMiddleware as service "auth".
- Assertions (c) + (d) now supply
`Authorization: Bearer integration-test-token` — full
production security shape exercised end-to-end with live
admin plugin subprocess.
PR-2 coordination needed: workflow-scenarios/92's app YAML must
reference an auth module (auth.jwt or similar) via
`infra-admin.config.auth_module`, and the Playwright spec must
send a valid Bearer token. Coordinated separately with
implementer-2.
Verified locally:
GOWORK=off go test ./module/ -run TestInfraAdmin -v
-> 24 unit tests PASS (21 prior + 3 new auth) + 1 integration
PASS with auth wired through all 4 assertions.
golangci-lint --new-from-rev=origin/main ./module/
-> 0 issues.
Per team-lead's PR-1 #791 blocking fix request 2026-05-27.
Co-Authored-By: Claude Opus 4.7 <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
1 participant
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
Ships a dynamic, proto-driven administrative interface for workflow's IaC subsystem, integrated with
workflow-plugin-adminvia the existing typedstep.admin_register_contributioncontract.infra.adminatmodule/infra_admin.go(modular framework) that mounts/api/infra-admin/*+/admin/infra-admin/*HTTP routes, resolvesiac.state/iac.providersiblings viaapp.GetService, and fires three single-step admin-contribution registration pipelines fromStart()viaengine.TriggerWorkflow.iac/admin/proto/infra_admin.proto(packageworkflow.iac.v1): 5 RPCs (ListResources / GetResource / ListResourceTypes / ListProviders / GenerateConfig), all read-only in v1. Mutation surface (TriggerAction) deferred to v1.1 per locked plan §Out of scope.iac/admin/handler/shared by module HTTP routes +wfctl infra admin *CLI subcommands.iac/admin/catalog/fields.gocovering all 13 typedinfra.*Configs with selectable-over-free-text CI audit; vendored proto + parity test atiac/admin/testdata/infra.protodetects drift.iac/admin/ui_dist/(resources.html, resource.html, new.html + external.js— no inline scripts, CSP-compliant).iac/admin/audit/.ResolveStateStore,LoadIaCProviderFromConfig,LoadAllIaCProvidersFromConfig— shared between host module and CLI.wfctl infra adminsubcommand family: list-resources, get-resource, list-types, list-providers, generate-config, audit-tail.module/infra_admin_integration_test.go) boots a real workflow engine + live workflow-plugin-admin gRPC subprocess + executes all 4 plan §Step 1-N assertions LIVE against the assembled router. ~3.86s on disposition.Design
See:
docs/plans/2026-05-27-infra-admin-dynamic-design.md(workspace; 7 adversarial design cycles converged).Implementation Plan
See:
docs/plans/2026-05-27-infra-admin-dynamic.md(workspace; Status: Locked 2026-05-27T15:34:13Z; manifest sha256=df31121e5ebf…).Scope Manifest (this PR's row)
feat/infra-admin-host-module-2026-05-27T1534Decision Records
decisions/0002-infra-admin-host-module.md) — engine-side carve-out justification (7 adversarial cycles independently proved each plugin-shaped placement infeasible).decisions/0003-infra-admin-t17-assertion-tier-amendment.md) — T17 assertion-tier amendment authorized then superseded by v4 implementation at 990a67a. Live execution restored.Changes (per-task summary)
cmd/wfctl/toiac/wfctlhelpers/so host module + CLI share one code path. Multi-provider helper added.workflow.iac.v1.provider_typecaptured at Init from WorkflowConfig (cycle-5 finding).infra.*Configs; freeform-audit CI test enforces selectable-over-free-text invariant.infra.proto+ parity test + Makefile target.resources.html(list+filter),resource.html(detail view + sensitive-redacted markers),new.html(form-builder with dependent dropdowns + YAML output).infra.adminmodule. ResolvesworkflowEngineat Start (registered post-app.Init per cycle-7 finding); usesAddRouteWithMiddleware+ explicitsecHdrs.Processwrap on every route.BuildFromConfigauto-inject; all 4 plan §Step 1-N assertions execute against live admin plugin subprocess.engine.gofactory registration.Verification
GOWORK=off go test ./...clean at HEAD (~22 module package tests + integration test PASS).GOWORK=off go build ./...clean.golangci-lint --new-from-rev=origin/main ./...0 issues.Test Plan
🤖 Generated with Claude Code