From c9e12a3573bb767fefcc9d209eca609c64665aa5 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:03:38 -0400 Subject: [PATCH 01/10] docs: design workflow ecosystem docs --- decisions/0048-wfctl-owned-go-api-docs.md | 34 +++ ...26-06-07-workflow-docs-ecosystem-design.md | 241 ++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 decisions/0048-wfctl-owned-go-api-docs.md create mode 100644 docs/plans/2026-06-07-workflow-docs-ecosystem-design.md diff --git a/decisions/0048-wfctl-owned-go-api-docs.md b/decisions/0048-wfctl-owned-go-api-docs.md new file mode 100644 index 000000000..e867f67ae --- /dev/null +++ b/decisions/0048-wfctl-owned-go-api-docs.md @@ -0,0 +1,34 @@ +# 0048. Generate Go API Docs With wfctl + +**Status:** Accepted +**Date:** 2026-06-07 +**Decision-makers:** workflow maintainers +**Related:** docs/plans/2026-06-07-workflow-docs-ecosystem-design.md + +## Context + +The public website already syncs Workflow and plugin Markdown, but it does not +publish Go API references for Workflow packages or plugin packages. The API +reference needs released-version awareness, registry-driven plugin discovery, +and Go package parsing. The website is a renderer and release surface; Workflow +already owns `wfctl`, plugin contracts, registry metadata, and Go package +semantics. + +## Decision + +We will put Go API extraction in `wfctl docs generate` and let +`gocodealone-website` call that command during docs sync. The generator uses Go +toolchain and stdlib package documentation APIs, emits Markdown plus version +metadata, and reads the registry snapshot to discover public plugin repos. + +Rejected alternatives: website-only Node generation would scatter Go package +rules into the renderer repo; linking only to pkg.go.dev would not provide a +coherent Workflow ecosystem reference or version navigation. + +## Consequences + +- The docs pipeline dogfoods `wfctl` and stays Go-native for Go semantics. +- Website sync gains a Go/wfctl dependency but remains mostly renderer logic. +- Versioning and plugin fallbacks can be tested at the CLI layer. +- If generation breaks, website can keep last committed docs while `wfctl` + reports per-repo warnings. diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem-design.md b/docs/plans/2026-06-07-workflow-docs-ecosystem-design.md new file mode 100644 index 000000000..220e00982 --- /dev/null +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem-design.md @@ -0,0 +1,241 @@ +--- +status: approved +area: ecosystem +owner: workflow +implementation_refs: [] +external_refs: + - "docs go-api generation" + - "Workflow 0.8 quirks cleanup" +verification: + last_checked: 2026-06-07 + commands: + - GOWORK=off go test ./cmd/wfctl ./config ./module ./plugin/... + - npm run sync-docs + - npm run build + result: planned +supersedes: [] +superseded_by: [] +--- + +# Workflow Docs Ecosystem Design + +Date: 2026-06-07 + +## Goal + +Make the public docs present Workflow as a coherent ecosystem: a YAML-first +application platform, a `wfctl`-assisted lifecycle tool, an extensible Go SDK, +and a plugin marketplace with released-version API reference. + +## Global Design Guidance + +Source: `docs/AGENT_GUIDE.md`, `docs/REPO_LAYOUT.md`, +`docs/plans/2026-04-25-workflow-ecosystem-audit-design.md`, +`decisions/0045-ci-generation-boundary.md`, and +`decisions/0048-wfctl-owned-go-api-docs.md`. + +| Guidance | Design response | +|---|---| +| Use `GOWORK=off` in this multi-repo workspace. | All Go verification commands specify `GOWORK=off`. | +| Keep durable examples under `example/`. | No new root examples; docs point at existing examples or generated reference pages. | +| `wfctl` is the lifecycle CLI and ecosystem audit surface. | Go API generation lives in `wfctl docs generate`, not website-only scripts. | +| Plugin/provider specifics live in plugins while core owns shared contracts. | Workflow docs cover SDK/contracts; plugin docs cover per-plugin packages and capabilities. | +| Design artifacts must remain traceable to implementation and verification. | Plan phases create PRs/releases and remove defect-ledger docs only after code fixes land. | + +## Reader Model + +The docs must serve two equally valid readers: + +1. **Application assemblers** who use YAML, plugin manifests, wfctl helpers, + and deployment docs without writing Go code. +2. **Application composers/extenders** who assemble a Workflow app and then add + app-local Go code, custom dynamic components, plugins, or provider contracts + as seen in production applications such as Buymywishlist and + workflow-compute. + +The public site should make both paths visible without forcing YAML-only users +through Go API pages or hiding SDK details from extension authors. + +## Information Architecture + +Reorganize generated docs into a stable narrative: + +- **Start Here**: what Workflow is, install, first app, core concepts. +- **Build Applications**: YAML-first path, wfctl-assisted path, manual path, + local test/debug, deploy, manage. +- **Extend With Code**: dynamic components, app-local Go code, plugin + authoring, contract testing, and production-style composition examples. +- **Ecosystem**: plugin catalog, provider capabilities, plugin status, release + and compatibility notes. +- **Reference**: config schema, wfctl CLI, Go API reference, plugin API + reference, migrations, and release notes. + +The website's Starlight sidebar should expose these lanes directly rather than +autogenerating a flat `workflow/` bucket that mixes tutorials, reference, +runbooks, and defect documents. + +## Go API Reference + +`wfctl docs generate` will produce Markdown API pages plus metadata JSON. The +generator owns Go-specific concerns: + +- discover package docs using Go stdlib parsing (`go/parser`, `go/doc`, + `go/ast`) and `go list` where needed; +- render package overview, exported constants, variables, functions, types, + methods, examples, import path, source links, module version, and warnings; +- read the website/registry plugin snapshot to enumerate public plugins; +- clone or reuse released tags for Workflow and plugin repositories; +- emit warnings per repo/package without failing the whole ecosystem build when + one plugin has no matching tag or no importable package; +- support a curated package allowlist first. + +Workflow curated packages start with: + +- `github.com/GoCodeAlone/workflow/plugin` +- `github.com/GoCodeAlone/workflow/plugin/sdk` +- `github.com/GoCodeAlone/workflow/plugin/external/sdk` +- `github.com/GoCodeAlone/workflow/config` +- `github.com/GoCodeAlone/workflow/cigen` +- `github.com/GoCodeAlone/workflow/module` +- `github.com/GoCodeAlone/workflow/pipeline` +- `github.com/GoCodeAlone/workflow/handlers` + +Plugin package coverage is curated per repo: root package when importable, SDK +or contract packages when present, and exported provider/step packages that are +not `internal`, `cmd`, `testdata`, generated-only, or private application +entrypoints. + +## Versioning + +Docs generate from released versions, not `main`. + +For Workflow pre-1.0, the public selector groups by minor line (`v0.8`, +`v0.9`) because every pre-1.0 minor may change compatibility. After v1.0, the +default selector groups by major line (`v1`, `v2`) with a metadata shape that +can expose minor selectors later. + +Generated output should be routeable as: + +- `/docs/reference/go/workflow/latest/` +- `/docs/reference/go/workflow/v0.8/` +- `/docs/reference/go/plugins//latest/` +- `/docs/reference/go/plugins///` + +The first website implementation may render version links rather than a custom +dropdown. The metadata must make a dropdown possible without regenerating page +content. + +## Defect-Ledger Docs And 0.8 Cleanup + +`docs/config-field-quirks.md` is accurate enough to be useful but harmful as +public doctrine. Known framework rough edges should become 0.8 fixes, then the +doc should be removed from public sync and replaced by migration/release notes. + +Fixes for the 0.8 path: + +- `http.server` accepts `port` as an alias for `address`, normalizing integer + ports to `":"`. +- `step.db_exec` and `step.db_query` accept `module` as an alias for + `database`. +- DB steps accept `args` as an alias for `params`. +- DB query mode accepts `many`/`one` as aliases for `list`/`single`. +- `step.request_parse` accepts `format: json` as equivalent to + `parse_body: true`. +- Register `step.response` as an alias for `step.json_response`. +- `step.json_response` avoids double-serializing template results that resolve + to JSON arrays/objects. +- `step.conditional` supports `if`/`then`/`else` alongside the existing + `field`/`routes` switch mode. +- `wfctl modernize` and schema/LSP suggestions should guide users away from + old or intuitive-but-noncanonical spellings. + +The inline trigger model is a real architecture choice. It should be explained +prominently in guide docs, not listed as a quirk. A future route-to-pipeline +compatibility feature can be planned separately if product direction requires +it. + +## Security Review + +Go API generation reads public repos and produces static Markdown. It must not +execute plugin code, run arbitrary package tests, or publish secrets. Clone +URLs come from trusted registry metadata but are still treated as untrusted +content: generation must cap clone depth, avoid shell interpolation, and redact +tokens from warnings. Website workflows use the least token scope needed to +fetch public repositories and open docs PRs. + +The 0.8 config alias work preserves existing behavior while accepting additional +input forms. Aliases must be normalized before execution so downstream modules +do not have multiple runtime interpretations. + +## Infrastructure Impact + +No cloud resources are created by the docs generator. Website CI/release +workflows gain Go/wfctl setup and may need caching for cloned repos or generated +docs. Public site size increases because API reference pages are committed and +packaged into `site.tar.gz`. + +Workflow 0.8 cleanup requires a minor release and downstream docs regeneration. +It may trigger plugin/website/registry sync workflows but should not require +production infrastructure approval. + +## Multi-Component Validation + +The minimum proof spans real boundaries: + +- `wfctl docs generate` runs against the released Workflow tag and at least + two plugin repos, writing Markdown and metadata. +- `gocodealone-website` `npm run sync-docs` invokes the real generator and + commits/render-previews generated API pages. +- Starlight build indexes the generated pages and version metadata. +- 0.8 alias fixes are verified through real config validation and representative + pipeline execution for HTTP, DB, request parsing, response, and conditional + steps. + +## Rollback + +- Docs generation rollback: revert website docs generator invocation and keep + the last committed Markdown snapshot. `sync-docs` must degrade to previous + behavior if generated API docs are absent. +- Workflow generator rollback: revert the `wfctl docs generate` PR and release + a patch if needed; website remains on committed reference docs until the next + successful sync. +- 0.8 behavior rollback: aliases are additive and can be disabled by reverting + the minor release before public docs remove the quirks page. The quirks page + is removed only after the release and regenerated docs confirm fixes. + +## Assumptions + +1. Public Workflow and plugin repos have release tags that match registry + versions often enough for released-version docs to be useful. +2. A curated package list is acceptable for the first API reference release. +3. Website release workflows can install/use Go and `wfctl` without expanding + permissions beyond repository contents and PR creation. +4. Pre-1.0 minor-line versioning is the right compatibility grouping for + Workflow until v1.0. +5. The quirks doc reflects active behavior and can be retired only after tests + prove the corresponding aliases/normalizations exist. + +## Non-Goals + +- Reimplementing pkg.go.dev completely. +- Executing plugin code during docs generation. +- Creating a custom search engine beyond Starlight/Pagefind indexing. +- Fixing every historical planning/audit document in `docs/plans/`. +- Adding route-to-pipeline syntax in the same PR as the alias cleanup unless + the implementation plan explicitly schedules it as a later phase. + +## Phases + +1. **Design and plan lock**: commit this design, ADR, adversarial reviews, and + implementation plan. +2. **Workflow 0.8 config cleanup**: fix the active quirks with tests and release + a minor version. +3. **Go API generator**: add `wfctl docs generate` with curated Workflow and + plugin package output plus version metadata. +4. **Website docs reorganization**: update Starlight sidebar/content layout, + invoke `wfctl docs generate`, render versioned API pages, and remove the + retired quirks page from public docs. +5. **Release and sync**: release Workflow, regenerate website docs, release the + website, and verify downstream multisite dispatch. + +Deferrals become later phases in this sequence rather than unscheduled backlog. From cc964f0da75733b2af3b911a08a3b7188007daf6 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:05:28 -0400 Subject: [PATCH 02/10] docs: review workflow docs ecosystem design --- ...7-workflow-docs-ecosystem-design-review.md | 45 +++++++++++++++++++ ...26-06-07-workflow-docs-ecosystem-design.md | 17 +++++++ 2 files changed, 62 insertions(+) create mode 100644 docs/plans/2026-06-07-workflow-docs-ecosystem-design-review.md diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem-design-review.md b/docs/plans/2026-06-07-workflow-docs-ecosystem-design-review.md new file mode 100644 index 000000000..58d7c5ebf --- /dev/null +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem-design-review.md @@ -0,0 +1,45 @@ +### Adversarial Review Report + +**Phase:** design +**Artifact:** docs/plans/2026-06-07-workflow-docs-ecosystem-design.md +**Status:** PASS + +**Findings (Critical):** +- None. + +**Findings (Important):** + +| id | class | loc | issue | recommendation | resolution | +|---|---|---|---|---|---| +| D1 | Missing failure modes / infra impact | `## Versioning` | Versioned API docs could grow unbounded if every Workflow/plugin version is committed to the website snapshot. | Add retention/pruning policy and keep first implementation bounded. | Resolved: design now publishes `latest` plus bounded recent Workflow lines; plugin version lines require reliable release metadata. | +| D2 | Assumptions / user-intent drift | `## Defect-Ledger Docs And 0.8 Cleanup` | Alias fixes could become permanent undocumented alternate schemas or be removed too quickly, either preserving quirks or breaking users. | Add explicit compatibility policy: canonical docs, modernize rewrite, warnings where practical, aliases supported through v1.0. | Resolved: design now includes compatibility policy. | + +**Findings (Minor):** + +| id | class | loc | issue | recommendation | +|---|---|---|---|---| +| D3 | Security / privacy | `## Security Review` | Registry repo URLs are trusted too broadly for first implementation. | Restrict first generator to public GoCodeAlone GitHub repos; defer wider third-party support to a separate trust-boundary review. | + +**Bug-class scan transcript:** + +| Class | Result | Note | +|---|---|---| +| Project-guidance conflicts | Clean | Design cites `docs/AGENT_GUIDE.md`, `docs/REPO_LAYOUT.md`, ecosystem audit, and CI boundary ADR; no conflict found. | +| Assumptions under attack | Finding | Alias support lifetime was underspecified; fixed by compatibility policy. | +| Repo-precedent conflicts | Clean | `wfctl` ownership matches existing lifecycle/audit precedent. | +| Artifact-class precedent | Clean | Docs sync already commits generated Markdown snapshots; design extends that artifact class. | +| YAGNI violations | Clean | Version dropdown is metadata-first; no custom UI required initially. | +| Missing failure modes | Finding | Version retention/pruning was missing; fixed. | +| Security / privacy | Finding | Repo trust boundary needed tightening; fixed for first implementation. | +| Infrastructure impact | Finding | Site-size growth needed bounded retention; fixed. | +| Multi-component validation | Clean | Design requires `wfctl docs generate` + website sync + Starlight build + representative config execution. | +| Rollback story | Clean | Design has rollback paths for generator, website sync, and 0.8 behavior changes. | +| Simpler alternative not considered | Clean | Design rejects website-only scripts and pkg.go.dev-only links with concrete trade-offs. | +| User-intent drift | Clean | Design covers holistic docs, released Go API docs, versioning, quirks cleanup, and doc removal. | +| Existence / runtime-validity | Clean | Plan must verify `wfctl` command surfaces and consumed website routes before relying on generated output. | + +**Options the author may not have considered:** +1. `pkgsite` static mirror: closer visual parity with pkg.go.dev, but heavier and less compatible with Starlight/navigation/version metadata. Keep as future option if stdlib Markdown output proves too thin. +2. Split docs IA and 0.8 cleanup into unrelated initiatives: lower PR risk, but it would leave a public defect doc in place while adding better API docs. Current phased design keeps order explicit. + +**Verdict reasoning:** PASS. The review found no Critical issues. Important issues were resolved in the design before this report was committed; remaining concerns are plan-level sizing and execution sequencing. diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem-design.md b/docs/plans/2026-06-07-workflow-docs-ecosystem-design.md index 220e00982..3ef72fd9b 100644 --- a/docs/plans/2026-06-07-workflow-docs-ecosystem-design.md +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem-design.md @@ -114,6 +114,13 @@ For Workflow pre-1.0, the public selector groups by minor line (`v0.8`, default selector groups by major line (`v1`, `v2`) with a metadata shape that can expose minor selectors later. +Generation is bounded. The default website snapshot publishes `latest` plus the +most recent three Workflow minor lines before v1.0, then `latest` plus the most +recent three major lines after v1.0. Older generated lines are pruned from the +website snapshot unless explicitly configured. Plugin API docs publish `latest` +first, then add version-line pages only when registry metadata exposes enough +release history to resolve them reliably. + Generated output should be routeable as: - `/docs/reference/go/workflow/latest/` @@ -131,6 +138,13 @@ content. public doctrine. Known framework rough edges should become 0.8 fixes, then the doc should be removed from public sync and replaced by migration/release notes. +Compatibility policy: 0.8 aliases are additive input normalization, not a second +canonical schema. Canonical docs continue to show one spelling. `wfctl +modernize` rewrites aliases to canonical keys. Validation should emit +non-blocking warnings where practical, but accepted aliases remain supported at +least through v1.0 because removing them before the first stable release would +undercut the purpose of fixing authoring ergonomics. + Fixes for the 0.8 path: - `http.server` accepts `port` as an alias for `address`, normalizing integer @@ -162,6 +176,9 @@ URLs come from trusted registry metadata but are still treated as untrusted content: generation must cap clone depth, avoid shell interpolation, and redact tokens from warnings. Website workflows use the least token scope needed to fetch public repositories and open docs PRs. +Repository inputs are constrained to public GitHub repos from GoCodeAlone-owned +registry entries in the first implementation. Wider third-party repository +support requires a separate trust-boundary review. The 0.8 config alias work preserves existing behavior while accepting additional input forms. Aliases must be normalized before execution so downstream modules From 30f400b7b775f48ffe91f55e496dab4d4b192b51 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:07:56 -0400 Subject: [PATCH 03/10] docs: plan workflow docs ecosystem --- .../2026-06-07-workflow-docs-ecosystem.md | 549 ++++++++++++++++++ 1 file changed, 549 insertions(+) create mode 100644 docs/plans/2026-06-07-workflow-docs-ecosystem.md diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem.md b/docs/plans/2026-06-07-workflow-docs-ecosystem.md new file mode 100644 index 000000000..7fc72761d --- /dev/null +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem.md @@ -0,0 +1,549 @@ +# Workflow Docs Ecosystem Implementation Plan + +> **For the implementing agent:** REQUIRED SUB-SKILL: Use autodev:executing-plans to implement this plan task-by-task. + +**Goal:** Build a coherent Workflow documentation system with 0.8 config-quirk cleanup, released-version Go API docs, plugin API docs, and website navigation that serves YAML assemblers and Go extenders. + +**Architecture:** Workflow owns behavior fixes and `wfctl docs generate`; the website consumes generated Markdown and version metadata during `sync-docs`. Public docs are reorganized into reader paths, and defect-ledger docs are removed only after verified fixes ship. + +**Tech Stack:** Go 1.26.4, `go/parser`, `go/doc`, `go/ast`, `go list`, Node 24, Astro Starlight, existing `scripts/sync-docs.mjs`, GitHub releases. + +**Base branch:** main + +--- + +## Scope Manifest + +**PR Count:** 3 +**Tasks:** 10 +**Estimated Lines of Change:** ~2200 + +**Out of scope:** +- Full pkg.go.dev UI parity. +- Executing plugin code during docs generation. +- Route-to-pipeline compatibility syntax. +- Third-party non-GoCodeAlone plugin repo ingestion. +- Removing alias support before v1.0. + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | +|------|-------|-------|--------| +| 1 | fix: normalize config authoring quirks | Task 1, Task 2, Task 3, Task 4 | fix/config-quirks-075 | +| 2 | feat: generate Go API docs with wfctl | Task 5, Task 6, Task 7 | feat/wfctl-go-docs | +| 3 | docs: reorganize website docs and API reference | Task 8, Task 9, Task 10 | feat/website-go-api-docs | + +**Status:** Draft + +## Global Design Guidance Mapping + +| Guidance | Plan wiring | +|---|---| +| `GOWORK=off` for Go commands | Every Go verification command includes `GOWORK=off`. | +| Prefer `wfctl` for lifecycle tooling | Task 5 creates `wfctl docs generate`; Task 8 dogfoods it from website sync. | +| Keep examples under `example/` | No new root examples; tests use temp dirs or existing fixtures. | +| Plugin/provider boundaries | Task 6 reads plugin registry metadata; generator does not execute plugin code. | +| Fix known product foibles before docs normalize them | Tasks 1-4 fix and verify quirks before Task 10 removes the public quirks page. | + +## Task 1: HTTP Server Port Alias And Registry Metadata + +**Files:** +- Modify: `plugins/http/factories.go` +- Modify: `plugins/http/schemas.go` +- Modify: `cmd/wfctl/type_registry.go` +- Test: `plugins/http/plugin_test.go` +- Test: `cmd/wfctl/validate_test.go` + +**Step 1: Write failing tests** + +Add tests proving `http.server` accepts: + +```yaml +config: + port: 8080 +``` + +Expected behavior: +- factory creates `module.NewStandardHTTPServer(name, ":8080")`; +- `address` still wins when both `address` and `port` exist; +- `wfctl validate` accepts `port` and type registry lists it as an alias. + +**Step 2: Verify RED** + +Run: + +```bash +GOWORK=off go test ./plugins/http ./cmd/wfctl -run 'TestHTTPServerPortAlias|TestValidateHTTPServerPortAlias' -count=1 +``` + +Expected: FAIL because `port` is ignored or validation still requires `address`. + +**Step 3: Implement** + +Add a small HTTP config normalizer in `plugins/http/factories.go`: +- if `address` is a non-empty string, use it; +- else if `port` is `int`, `int64`, `float64`, or numeric string, convert to `":"`; +- reject invalid ports with a clear factory error. + +Update schema/type registry metadata so docs/completions know `port` is accepted but `address` is canonical. + +**Step 4: Verify GREEN** + +Run: + +```bash +GOWORK=off go test ./plugins/http ./cmd/wfctl -run 'TestHTTPServerPortAlias|TestValidateHTTPServerPortAlias' -count=1 +``` + +Expected: PASS. + +**Step 5: Regression proof** + +Temporarily revert only the production normalizer. Re-run the RED command. + +Expected: FAIL on `port` alias. Restore fix and re-run; Expected: PASS. + +**Rollback:** revert this task commit; configs using canonical `address` remain valid. + +## Task 2: Database Step Alias Normalization + +**Files:** +- Modify: `module/pipeline_step_db_query.go` +- Modify: `module/pipeline_step_db_exec.go` +- Modify: `module/pipeline_step_db_query_cached.go` if it shares the same authoring surface. +- Test: `module/pipeline_step_db_query_test.go` +- Test: `module/pipeline_step_db_exec_test.go` +- Test: `cmd/wfctl/modernize_test.go` + +**Step 1: Write failing tests** + +Add tests proving: +- `module` aliases `database` for `step.db_query`, `step.db_exec`, and cached query if applicable; +- `args` aliases `params`; +- `mode: many` normalizes to `list`; +- `mode: one` normalizes to `single`; +- canonical keys win when both forms exist. + +**Step 2: Verify RED** + +Run: + +```bash +GOWORK=off go test ./module ./cmd/wfctl -run 'TestDB(Query|Exec).*Alias|TestModernize.*DB' -count=1 +``` + +Expected: FAIL on missing `database`, ignored `args`, or invalid mode. + +**Step 3: Implement** + +Create unexported helpers near DB step code: + +```go +func configStringAlias(cfg map[string]any, canonical string, aliases ...string) string +func configListAlias(cfg map[string]any, canonical string, aliases ...string) []string +func normalizeDBMode(mode string) string +``` + +Use helpers from query/exec factories. Keep runtime fields canonical. + +Extend `modernize.AllRules()` so `wfctl modernize --fix` rewrites `module` to `database`, `args` to `params`, and `many`/`one` to `list`/`single`. + +**Step 4: Verify GREEN** + +Run: + +```bash +GOWORK=off go test ./module ./cmd/wfctl -run 'TestDB(Query|Exec).*Alias|TestModernize.*DB' -count=1 +``` + +Expected: PASS. + +**Step 5: Regression proof** + +Revert DB production helper usage, keep tests, re-run targeted command. + +Expected: FAIL on alias tests. Restore and re-run; Expected: PASS. + +**Rollback:** revert this task commit; canonical DB configs remain valid. + +## Task 3: Request, Response, And Conditional Ergonomics + +**Files:** +- Modify: `module/pipeline_step_request_parse.go` +- Modify: `module/pipeline_step_json_response.go` +- Modify: `module/pipeline_step_conditional.go` +- Modify: `plugins/pipelinesteps/plugin.go` +- Modify: `handlers/testhelpers_test.go` +- Modify: `cmd/wfctl/type_registry.go` +- Test: `module/pipeline_step_request_parse_test.go` +- Test: `module/pipeline_step_json_response_test.go` +- Test: `module/pipeline_step_conditional_test.go` +- Test: `plugins/pipelinesteps/plugin_test.go` + +**Step 1: Write failing tests** + +Add tests proving: +- `step.request_parse` with `format: json` sets `parseBody=true`; +- `step.response` is registered and behaves like `step.json_response`; +- template body strings resolving to JSON arrays/objects encode as arrays/objects, not quoted JSON strings; +- `step.conditional` supports: + +```yaml +config: + if: "{{ .status == \"active\" }}" + then: proceed + else: reject +``` + +Expected conditional output includes `next_step`. + +**Step 2: Verify RED** + +Run: + +```bash +GOWORK=off go test ./module ./plugins/pipelinesteps -run 'TestRequestParseFormatAlias|TestJSONResponseTemplateRawJSON|TestConditionalIfThenElse|TestStepResponseAlias' -count=1 +``` + +Expected: FAIL because aliases/mode are absent. + +**Step 3: Implement** + +Implementation rules: +- `format: json` and `format: form` imply body parsing; JSON remains content-type/JSON parsing behavior. +- `step.response` maps to `NewJSONResponseStepFactory()` in plugin manifest/factory/type registry. +- JSON response template output: if resolved string is valid JSON object/array, unmarshal to `any` before encoding; leave scalars and invalid JSON as strings. +- Conditional: add mode fields `ifExpr`, `thenStep`, `elseStep`; if `if` exists, evaluate template/expr result as bool/string truthy; route to then/else. + +**Step 4: Verify GREEN** + +Run: + +```bash +GOWORK=off go test ./module ./plugins/pipelinesteps -run 'TestRequestParseFormatAlias|TestJSONResponseTemplateRawJSON|TestConditionalIfThenElse|TestStepResponseAlias' -count=1 +``` + +Expected: PASS. + +**Step 5: Regression proof** + +Revert production changes for request/response/conditional, keep tests. + +Expected: targeted command FAIL. Restore and re-run; Expected: PASS. + +**Rollback:** revert this task commit; canonical pipeline configs remain valid. + +## Task 4: Retire Public Quirks Doc In Workflow Source + +**Files:** +- Delete: `docs/config-field-quirks.md` +- Modify: `docs/dsl-reference.md` +- Modify: `docs/tutorials/building-apps-with-workflow.md` +- Modify: `docs/WFCTL.md` +- Test: `cmd/wfctl/modernize_test.go` + +**Step 1: Write failing doc-sync guard** + +Add/extend a test that fails when `config-field-quirks.md` is still considered a manual public doc. + +Run: + +```bash +GOWORK=off go test ./cmd/wfctl -run TestModernizeConfigQuirkAliases -count=1 +``` + +Expected: FAIL until modernize rules and docs cleanup are in place. + +**Step 2: Update docs** + +Replace defect-ledger content with: +- canonical examples in `dsl-reference.md`; +- a short `WFCTL.md` section for `wfctl modernize` aliases; +- tutorial explanation that inline pipeline triggers are intentional. + +Delete `docs/config-field-quirks.md`. + +**Step 3: Verify** + +Run: + +```bash +GOWORK=off go test ./cmd/wfctl ./module ./plugins/http ./plugins/pipelinesteps -count=1 +GOWORK=off go test ./... -count=1 +``` + +Expected: PASS. + +**Rollback:** restore the deleted doc and revert docs edits if the behavior release is reverted. + +## Task 5: Add `wfctl docs generate` Command Skeleton And Workflow Packages + +**Files:** +- Create: `cmd/wfctl/docs_generate.go` +- Create: `cmd/wfctl/docs_generate_test.go` +- Modify: `cmd/wfctl/main.go` +- Modify: `docs/WFCTL.md` + +**Step 1: Write failing CLI tests** + +Test: +- `wfctl docs generate --help` exposes flags; +- running against the current repo with `--source . --out --module github.com/GoCodeAlone/workflow --version v0.75.0 --packages plugin,plugin/sdk,plugin/external/sdk` writes Markdown and `versions.json`; +- output contains import path, package synopsis, exported types/functions, source link, and warning list. + +**Step 2: Verify RED** + +Run: + +```bash +GOWORK=off go test ./cmd/wfctl -run TestDocsGenerate -count=1 +``` + +Expected: FAIL because command is absent. + +**Step 3: Implement minimal generator** + +Use Go stdlib: +- `go list -json` for package dirs/import paths; +- `parser.ParseDir` + `doc.New` for docs; +- Markdown renderer with stable headings; +- metadata JSON with `schemaVersion`, `generatedAt`, `subject`, `versions`, `packages`, `warnings`. + +**Step 4: Verify GREEN** + +Run: + +```bash +GOWORK=off go test ./cmd/wfctl -run TestDocsGenerate -count=1 +GOWORK=off go run ./cmd/wfctl docs generate --source . --out "$TMPDIR/workflow-go-docs" --module github.com/GoCodeAlone/workflow --version v0.75.0 --packages plugin,plugin/sdk,plugin/external/sdk +``` + +Expected: PASS; output dir contains Markdown and metadata. + +**Rollback:** remove command and docs; existing `wfctl docs` behavior remains unchanged. + +## Task 6: Add Released-Tag Plugin API Generation + +**Files:** +- Modify: `cmd/wfctl/docs_generate.go` +- Modify: `cmd/wfctl/docs_generate_test.go` +- Create: `cmd/wfctl/testdata/docs-registry.json` + +**Step 1: Write failing registry tests** + +Test with two local fixture repos or testdata: +- registry entries discover `repository`, `source`, `version`, `name`; +- generated routes are `plugins//latest/`; +- missing matching tag creates warning but does not fail whole generation; +- non-GoCodeAlone or non-GitHub repos are skipped with a trust-boundary warning. + +**Step 2: Verify RED** + +Run: + +```bash +GOWORK=off go test ./cmd/wfctl -run TestDocsGenerateRegistryPlugins -count=1 +``` + +Expected: FAIL because registry mode is absent. + +**Step 3: Implement** + +Add flags: +- `--registry ` +- `--cache-dir ` +- `--subjects workflow,plugins` +- `--max-version-lines 3` + +Use non-shell `git` invocations via `exec.CommandContext`. Clone depth 1 when +possible; checkout exact release tag. Restrict first implementation to +`https://github.com/GoCodeAlone/`. + +**Step 4: Verify GREEN** + +Run: + +```bash +GOWORK=off go test ./cmd/wfctl -run TestDocsGenerateRegistryPlugins -count=1 +``` + +Expected: PASS with warnings asserted. + +**Rollback:** keep workflow-only generation; disable plugin registry flag in website sync. + +## Task 7: Release Workflow Minor Version + +**Files:** +- No source files unless release notes are stored in repo. + +**Step 1: Pre-release verification** + +Run: + +```bash +GOWORK=off go test ./cmd/wfctl ./config ./module ./plugin/... ./plugins/http ./plugins/pipelinesteps -count=1 +GOWORK=off go test ./... -count=1 +``` + +Expected: PASS. + +**Step 2: Merge PRs 1 and 2** + +After CI/review green, admin-merge PRs in order. + +**Step 3: Trigger minor release** + +Run the workflow release automation from main with minor bump. + +Expected: new Workflow release tag after `v0.74.7`, e.g. `v0.75.0`, publishes `wfctl` assets. + +**Rollback:** if release fails before publish, rerun after fix; if published with regression, release patch reverting the bad commits. + +## Task 8: Website Sync Invokes `wfctl docs generate` + +**Files in `gocodealone-website`:** +- Modify: `scripts/sync-docs.mjs` +- Modify: `package.json` +- Modify: `.github/workflows/sync-docs.yml` +- Modify: `.github/workflows/ci.yml` +- Test: `test/sync-docs.test.mjs` + +**Step 1: Write failing website tests** + +Test: +- generated Go docs are expected under `docs/site/src/content/docs/reference/go/...`; +- sync removes stale generated API docs outside retention; +- `config-field-quirks.md` is not synced. + +**Step 2: Verify RED** + +Run: + +```bash +npm test -- test/sync-docs.test.mjs +``` + +Expected: FAIL because no Go-doc generation exists and quirks doc is still allowed. + +**Step 3: Implement** + +Update `sync-docs.mjs` to: +- call `wfctl docs generate` after Markdown sync; +- pass `src/data/plugins.json` as registry input; +- write generated API docs under `reference/go`; +- tolerate generator warnings but fail if workflow core generation emits no pages; +- exclude `config-field-quirks.md` from `WORKFLOW_MANUAL_DOCS`. + +Update workflows to install/use the newly released `wfctl` version. + +**Step 4: Verify GREEN** + +Run: + +```bash +GITHUB_TOKEN=$(gh auth token) npm run sync-docs +npm test -- test/sync-docs.test.mjs +npm run build +``` + +Expected: PASS; generated reference pages are present and Starlight builds. + +**Rollback:** revert website sync invocation; committed generated docs remain last-known-good. + +## Task 9: Website Navigation And Version Metadata + +**Files in `gocodealone-website`:** +- Modify: `docs/site/astro.config.mjs` +- Create/Modify: `docs/site/src/content/docs/reference/index.md` +- Create/Modify: `docs/site/src/content/docs/start-here/index.md` if needed +- Modify: generated docs metadata location. +- Test: `test/sync-docs.test.mjs` + +**Step 1: Write failing navigation tests** + +Assert docs contain sections/routes for: +- Start Here +- Build Applications +- Extend With Code +- Ecosystem +- Reference +- Go API reference version metadata + +**Step 2: Verify RED** + +Run: + +```bash +npm test -- test/sync-docs.test.mjs +``` + +Expected: FAIL because sidebar/content is still flat. + +**Step 3: Implement** + +Use Starlight sidebar groups rather than one flat autogenerate bucket. Keep old +routes where possible; add landing pages that link both wfctl-assisted and +manual paths. + +Render version links from generated metadata in Markdown or a small docs data +file. Do not build a heavy custom dropdown unless route metadata proves stable. + +**Step 4: Verify GREEN** + +Run: + +```bash +npm test -- test/sync-docs.test.mjs +npm run build +``` + +Expected: PASS; Pagefind indexes generated reference pages. + +**Rollback:** restore previous sidebar and keep generated pages reachable by direct URL. + +## Task 10: Regenerate Website Docs, Release Website + +**Files in `gocodealone-website`:** +- Generated Markdown under `docs/site/src/content/docs/` +- `src/data/plugins.json` if registry changed + +**Step 1: Regenerate** + +Run: + +```bash +GITHUB_TOKEN=$(gh auth token) npm run sync-plugins +GITHUB_TOKEN=$(gh auth token) npm run sync-docs +npm run lint +npm test +npm run validate-configs +npm run build +npm run package:site +``` + +Expected: +- no `docs/site/src/content/docs/workflow/config-field-quirks.md`; +- Go API pages exist under `reference/go`; +- build exits 0. + +**Step 2: Open and monitor PR 3** + +Create website PR, add `copilot-pull-request-reviewer`, invoke `autodev:pr-monitoring`, fix feedback, merge after green checks. + +**Step 3: Release website** + +Tag next website patch release after merge and monitor release workflow. + +Expected: +- GitHub release contains `site.tar.gz`; +- multisite repository dispatch succeeds. + +**Rollback:** delete/revert website tag only before release completes; after publish, release a patch restoring previous docs snapshot. + +## Post-Merge Release Order + +1. Merge PR 1 (config cleanup), verify CI. +2. Merge PR 2 (docs generator), verify CI. +3. Release Workflow minor (`v0.75.0` if latest remains `v0.74.7`). +4. Merge PR 3 (website docs/navigation), verify CI. +5. Release website patch and verify multisite dispatch. From b97ae44e7adea763dd4a6429519ec9e32c5958f8 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:09:36 -0400 Subject: [PATCH 04/10] docs: review workflow docs ecosystem plan --- ...-07-workflow-docs-ecosystem-plan-review.md | 55 +++++++++++++++++++ .../2026-06-07-workflow-docs-ecosystem.md | 46 +++------------- 2 files changed, 64 insertions(+), 37 deletions(-) create mode 100644 docs/plans/2026-06-07-workflow-docs-ecosystem-plan-review.md diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem-plan-review.md b/docs/plans/2026-06-07-workflow-docs-ecosystem-plan-review.md new file mode 100644 index 000000000..17b5f7d35 --- /dev/null +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem-plan-review.md @@ -0,0 +1,55 @@ +### Adversarial Review Report + +**Phase:** plan +**Artifact:** docs/plans/2026-06-07-workflow-docs-ecosystem.md +**Status:** PASS + +**Findings (Critical):** +- None. + +**Findings (Important):** + +| id | class | loc | issue | recommendation | resolution | +|---|---|---|---|---|---| +| P1 | Scope manifest / over-decomposition | `Task 7` in first draft | Workflow release was modeled as an implementation task assigned to PR 2, but release orchestration is not a code PR and violates the manifest rule that every task maps to PR rows. | Remove release as numbered task; keep release order in post-merge checklist. | Resolved: plan now has 9 implementation tasks; releases live in `## Post-Merge Release Order`. | +| P2 | Existence / runtime-validity | `Task 1` files | First draft named `plugins/http/factories.go`, which does not exist. | Verify files and change target to `plugins/http/modules.go`. | Resolved: file target corrected and existence checked. | + +**Findings (Minor):** + +| id | class | loc | issue | recommendation | +|---|---|---|---|---| +| P3 | Verification-class mismatch | Task 9 | Website release verification depends on external multisite dispatch timing. | Keep bounded polling and report dispatch run URL; do not claim deployment without observed successful run. | + +**Bug-class scan transcript:** + +| Class | Result | Note | +|---|---|---| +| Project-guidance conflicts | Clean | Plan wires `GOWORK=off`, examples policy, wfctl ownership, and plugin boundaries. | +| Assumptions under attack | Clean | Version tag and plugin warning fallbacks are explicit. | +| Repo-precedent conflicts | Clean | `wfctl docs` command pattern matches existing `cmd/wfctl/docs.go` command surface. | +| Artifact-class precedent | Finding | Non-existent HTTP file caught and fixed. | +| YAGNI violations | Clean | Custom dropdown deferred; metadata/link rendering first. | +| Missing failure modes | Clean | Generator warnings, stale doc pruning, rollback, and release failure modes are listed. | +| Security / privacy | Clean | First implementation constrains repo ingestion to public GoCodeAlone GitHub repos and avoids plugin execution. | +| Infrastructure impact | Clean | No cloud resources; workflow/site release side effects are noted. | +| Multi-component validation | Clean | Plan exercises workflow code, CLI generator, website sync, Starlight build, release, and multisite dispatch. | +| Rollback story | Clean | Runtime-affecting tasks include rollback notes; release rollback is in post-merge order. | +| Simpler alternative not considered | Clean | Design already rejects website-only and pkg.go.dev-only alternatives. | +| User-intent drift | Clean | Plan covers holistic docs, Go docs, released versions, quirks cleanup, doc removal, and releases. | +| Existence / runtime-validity | Finding | Bad file path caught; plan also requires command/help tests before website consumes generator. | +| Over/under-decomposition | Finding | Release orchestration task removed from manifest; remaining tasks are reviewable slices. | +| Verification-class mismatch | Minor | Task 9 requires observed multisite dispatch result to support completion claims. | +| Auth/authz chain composition | Clean | No auth chain changes. | +| Hidden serial dependencies | Clean | PR order is serial where needed: behavior cleanup → generator → website. | +| Missing rollback wiring | Clean | Rollbacks are included per runtime-affecting task. | +| Missing integration proof | Clean | Integration proof crosses CLI generator and website build. | +| Infrastructure verification mismatch | Clean | No IaC apply/plan changes. | +| Plugin-loader runtime layout | Clean | No external plugin process loading. | +| Config-validation schema rules | Clean | Config alias tasks include wfctl validation and representative execution. | +| Identifier / naming-convention match | Clean | Canonical spellings remain documented; aliases normalize into existing fields. | + +**Options the author may not have considered:** +1. Ship only the quirks cleanup first and defer docs generator: lower first-PR risk, but user explicitly asked to continue phases automatically; the plan keeps PR order serial while preserving full scope. +2. Put website IA in a separate fourth PR from generated Go docs: easier review, but generated docs need navigation to be useful. Task 8 keeps first UI minimal. + +**Verdict reasoning:** PASS. No Critical findings. Important findings were resolved in the plan before this report was committed; remaining risk is execution sizing and release timing, handled by serial PR order and bounded verification. diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem.md b/docs/plans/2026-06-07-workflow-docs-ecosystem.md index 7fc72761d..06d58b000 100644 --- a/docs/plans/2026-06-07-workflow-docs-ecosystem.md +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem.md @@ -15,7 +15,7 @@ ## Scope Manifest **PR Count:** 3 -**Tasks:** 10 +**Tasks:** 9 **Estimated Lines of Change:** ~2200 **Out of scope:** @@ -30,8 +30,8 @@ | PR # | Title | Tasks | Branch | |------|-------|-------|--------| | 1 | fix: normalize config authoring quirks | Task 1, Task 2, Task 3, Task 4 | fix/config-quirks-075 | -| 2 | feat: generate Go API docs with wfctl | Task 5, Task 6, Task 7 | feat/wfctl-go-docs | -| 3 | docs: reorganize website docs and API reference | Task 8, Task 9, Task 10 | feat/website-go-api-docs | +| 2 | feat: generate Go API docs with wfctl | Task 5, Task 6 | feat/wfctl-go-docs | +| 3 | docs: reorganize website docs and API reference | Task 7, Task 8, Task 9 | feat/website-go-api-docs | **Status:** Draft @@ -40,15 +40,15 @@ | Guidance | Plan wiring | |---|---| | `GOWORK=off` for Go commands | Every Go verification command includes `GOWORK=off`. | -| Prefer `wfctl` for lifecycle tooling | Task 5 creates `wfctl docs generate`; Task 8 dogfoods it from website sync. | +| Prefer `wfctl` for lifecycle tooling | Task 5 creates `wfctl docs generate`; Task 7 dogfoods it from website sync. | | Keep examples under `example/` | No new root examples; tests use temp dirs or existing fixtures. | | Plugin/provider boundaries | Task 6 reads plugin registry metadata; generator does not execute plugin code. | -| Fix known product foibles before docs normalize them | Tasks 1-4 fix and verify quirks before Task 10 removes the public quirks page. | +| Fix known product foibles before docs normalize them | Tasks 1-4 fix and verify quirks before Task 9 removes the public quirks page. | ## Task 1: HTTP Server Port Alias And Registry Metadata **Files:** -- Modify: `plugins/http/factories.go` +- Modify: `plugins/http/modules.go` - Modify: `plugins/http/schemas.go` - Modify: `cmd/wfctl/type_registry.go` - Test: `plugins/http/plugin_test.go` @@ -371,35 +371,7 @@ Expected: PASS with warnings asserted. **Rollback:** keep workflow-only generation; disable plugin registry flag in website sync. -## Task 7: Release Workflow Minor Version - -**Files:** -- No source files unless release notes are stored in repo. - -**Step 1: Pre-release verification** - -Run: - -```bash -GOWORK=off go test ./cmd/wfctl ./config ./module ./plugin/... ./plugins/http ./plugins/pipelinesteps -count=1 -GOWORK=off go test ./... -count=1 -``` - -Expected: PASS. - -**Step 2: Merge PRs 1 and 2** - -After CI/review green, admin-merge PRs in order. - -**Step 3: Trigger minor release** - -Run the workflow release automation from main with minor bump. - -Expected: new Workflow release tag after `v0.74.7`, e.g. `v0.75.0`, publishes `wfctl` assets. - -**Rollback:** if release fails before publish, rerun after fix; if published with regression, release patch reverting the bad commits. - -## Task 8: Website Sync Invokes `wfctl docs generate` +## Task 7: Website Sync Invokes `wfctl docs generate` **Files in `gocodealone-website`:** - Modify: `scripts/sync-docs.mjs` @@ -450,7 +422,7 @@ Expected: PASS; generated reference pages are present and Starlight builds. **Rollback:** revert website sync invocation; committed generated docs remain last-known-good. -## Task 9: Website Navigation And Version Metadata +## Task 8: Website Navigation And Version Metadata **Files in `gocodealone-website`:** - Modify: `docs/site/astro.config.mjs` @@ -501,7 +473,7 @@ Expected: PASS; Pagefind indexes generated reference pages. **Rollback:** restore previous sidebar and keep generated pages reachable by direct URL. -## Task 10: Regenerate Website Docs, Release Website +## Task 9: Regenerate Website Docs, Release Website **Files in `gocodealone-website`:** - Generated Markdown under `docs/site/src/content/docs/` From 1305a7642dadcca4389f60875c2fc5b901d94375 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:12:28 -0400 Subject: [PATCH 05/10] chore: lock scope for workflow docs ecosystem --- ...workflow-docs-ecosystem-alignment-check.md | 42 +++++++++++++++++++ .../2026-06-07-workflow-docs-ecosystem.md | 20 ++++----- ...6-07-workflow-docs-ecosystem.md.scope-lock | 1 + 3 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 docs/plans/2026-06-07-workflow-docs-ecosystem-alignment-check.md create mode 100644 docs/plans/2026-06-07-workflow-docs-ecosystem.md.scope-lock diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem-alignment-check.md b/docs/plans/2026-06-07-workflow-docs-ecosystem-alignment-check.md new file mode 100644 index 000000000..a464e90a7 --- /dev/null +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem-alignment-check.md @@ -0,0 +1,42 @@ +# Workflow Docs Ecosystem Alignment Check + +Date: 2026-06-07 + +Design: `docs/plans/2026-06-07-workflow-docs-ecosystem-design.md` +Plan: `docs/plans/2026-06-07-workflow-docs-ecosystem.md` + +## Alignment Report + +**Status:** PASS + +**Coverage:** + +| Design Requirement | Plan Task(s) | Status | +|---|---|---| +| Fix known config field quirks before removing the public defect-ledger doc. | Task 1, Task 2, Task 3, Task 4 | Covered | +| Preserve additive aliases through v1.0 while documenting canonical fields and modernizing old forms. | Task 1, Task 2, Task 3, Task 4 | Covered | +| Generate Go API reference from released Workflow versions with curated package coverage. | Task 5 | Covered | +| Generate plugin API reference from released public GoCodeAlone plugin repositories without executing plugin code. | Task 6 | Covered | +| Dogfood `wfctl docs generate` from website sync and keep generated docs current. | Task 7, Task 9 | Covered | +| Reorganize public docs around Start Here, Build Applications, Extend With Code, Ecosystem, and Reference lanes. | Task 8 | Covered | +| Support versioned docs metadata and bounded latest/version-line output. | Task 5, Task 6, Task 8 | Covered | +| Remove `config-field-quirks.md` from public docs only after behavior fixes are verified. | Task 4, Task 7, Task 9 | Covered | +| Release Workflow first, then regenerate and release the website. | Post-Merge Release Order, Task 9 | Covered | + +**Scope Check:** + +| Plan Task | Design Requirement | Status | +|---|---|---| +| Task 1 | HTTP server alias cleanup and canonical schema guidance. | Justified | +| Task 2 | DB step alias cleanup and `wfctl modernize` canonicalization. | Justified | +| Task 3 | Request/response/conditional authoring ergonomics. | Justified | +| Task 4 | Retire public quirks doc after verified fixes. | Justified | +| Task 5 | Workflow released-version Go API generation. | Justified | +| Task 6 | Plugin released-version API generation under trust boundary. | Justified | +| Task 7 | Website sync dogfoods generated API docs. | Justified | +| Task 8 | Website IA and version metadata. | Justified | +| Task 9 | Regeneration, PR monitoring, and release verification. | Justified | + +**Manifest Check:** PASS via `plan-scope-check.sh --plan`. + +**Drift Items:** None. diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem.md b/docs/plans/2026-06-07-workflow-docs-ecosystem.md index 06d58b000..2e27424c3 100644 --- a/docs/plans/2026-06-07-workflow-docs-ecosystem.md +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem.md @@ -33,7 +33,7 @@ | 2 | feat: generate Go API docs with wfctl | Task 5, Task 6 | feat/wfctl-go-docs | | 3 | docs: reorganize website docs and API reference | Task 7, Task 8, Task 9 | feat/website-go-api-docs | -**Status:** Draft +**Status:** Locked 2026-06-07T04:11:52Z ## Global Design Guidance Mapping @@ -45,7 +45,7 @@ | Plugin/provider boundaries | Task 6 reads plugin registry metadata; generator does not execute plugin code. | | Fix known product foibles before docs normalize them | Tasks 1-4 fix and verify quirks before Task 9 removes the public quirks page. | -## Task 1: HTTP Server Port Alias And Registry Metadata +### Task 1: HTTP Server Port Alias And Registry Metadata **Files:** - Modify: `plugins/http/modules.go` @@ -105,7 +105,7 @@ Expected: FAIL on `port` alias. Restore fix and re-run; Expected: PASS. **Rollback:** revert this task commit; configs using canonical `address` remain valid. -## Task 2: Database Step Alias Normalization +### Task 2: Database Step Alias Normalization **Files:** - Modify: `module/pipeline_step_db_query.go` @@ -166,7 +166,7 @@ Expected: FAIL on alias tests. Restore and re-run; Expected: PASS. **Rollback:** revert this task commit; canonical DB configs remain valid. -## Task 3: Request, Response, And Conditional Ergonomics +### Task 3: Request, Response, And Conditional Ergonomics **Files:** - Modify: `module/pipeline_step_request_parse.go` @@ -233,7 +233,7 @@ Expected: targeted command FAIL. Restore and re-run; Expected: PASS. **Rollback:** revert this task commit; canonical pipeline configs remain valid. -## Task 4: Retire Public Quirks Doc In Workflow Source +### Task 4: Retire Public Quirks Doc In Workflow Source **Files:** - Delete: `docs/config-field-quirks.md` @@ -276,7 +276,7 @@ Expected: PASS. **Rollback:** restore the deleted doc and revert docs edits if the behavior release is reverted. -## Task 5: Add `wfctl docs generate` Command Skeleton And Workflow Packages +### Task 5: Add `wfctl docs generate` Command Skeleton And Workflow Packages **Files:** - Create: `cmd/wfctl/docs_generate.go` @@ -322,7 +322,7 @@ Expected: PASS; output dir contains Markdown and metadata. **Rollback:** remove command and docs; existing `wfctl docs` behavior remains unchanged. -## Task 6: Add Released-Tag Plugin API Generation +### Task 6: Add Released-Tag Plugin API Generation **Files:** - Modify: `cmd/wfctl/docs_generate.go` @@ -371,7 +371,7 @@ Expected: PASS with warnings asserted. **Rollback:** keep workflow-only generation; disable plugin registry flag in website sync. -## Task 7: Website Sync Invokes `wfctl docs generate` +### Task 7: Website Sync Invokes `wfctl docs generate` **Files in `gocodealone-website`:** - Modify: `scripts/sync-docs.mjs` @@ -422,7 +422,7 @@ Expected: PASS; generated reference pages are present and Starlight builds. **Rollback:** revert website sync invocation; committed generated docs remain last-known-good. -## Task 8: Website Navigation And Version Metadata +### Task 8: Website Navigation And Version Metadata **Files in `gocodealone-website`:** - Modify: `docs/site/astro.config.mjs` @@ -473,7 +473,7 @@ Expected: PASS; Pagefind indexes generated reference pages. **Rollback:** restore previous sidebar and keep generated pages reachable by direct URL. -## Task 9: Regenerate Website Docs, Release Website +### Task 9: Regenerate Website Docs, Release Website **Files in `gocodealone-website`:** - Generated Markdown under `docs/site/src/content/docs/` diff --git a/docs/plans/2026-06-07-workflow-docs-ecosystem.md.scope-lock b/docs/plans/2026-06-07-workflow-docs-ecosystem.md.scope-lock new file mode 100644 index 000000000..4883b1231 --- /dev/null +++ b/docs/plans/2026-06-07-workflow-docs-ecosystem.md.scope-lock @@ -0,0 +1 @@ +307afc73d443baec2a91323b0194237a8ea28dd77cab8c48ef8f6439cd2442b7 From 6fb793f7ac7ad6b35c883fd0352f0a17c3e29868 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:18:09 -0400 Subject: [PATCH 06/10] fix: accept http server port alias --- cmd/wfctl/type_registry.go | 2 +- cmd/wfctl/type_registry_test.go | 16 +++++++ module/http_server.go | 2 +- module/reflect_validation_test.go | 2 +- plugins/http/modules.go | 77 +++++++++++++++++++++++++++++-- plugins/http/plugin_test.go | 53 +++++++++++++++++++++ plugins/http/schemas.go | 3 +- schema/module_schema.go | 3 +- schema/schema_test.go | 22 +++++++-- schema/validate.go | 16 +++++++ 10 files changed, 183 insertions(+), 13 deletions(-) diff --git a/cmd/wfctl/type_registry.go b/cmd/wfctl/type_registry.go index c42ca1e70..6a0e2b2a5 100644 --- a/cmd/wfctl/type_registry.go +++ b/cmd/wfctl/type_registry.go @@ -87,7 +87,7 @@ func KnownModuleTypes() map[string]ModuleTypeInfo { Type: "http.server", Plugin: "http", Stateful: false, - ConfigKeys: []string{"address", "readTimeout", "writeTimeout", "idleTimeout"}, + ConfigKeys: []string{"address", "port", "readTimeout", "writeTimeout", "idleTimeout"}, }, "http.client": { Type: "http.client", diff --git a/cmd/wfctl/type_registry_test.go b/cmd/wfctl/type_registry_test.go index 0160cfd9e..86c1ff213 100644 --- a/cmd/wfctl/type_registry_test.go +++ b/cmd/wfctl/type_registry_test.go @@ -78,6 +78,22 @@ func TestKnownModuleTypesStateful(t *testing.T) { } } +func TestKnownModuleTypesHTTPServerListsPortAlias(t *testing.T) { + info, ok := KnownModuleTypes()["http.server"] + if !ok { + t.Fatal("http.server not found") + } + keys := make(map[string]bool, len(info.ConfigKeys)) + for _, key := range info.ConfigKeys { + keys[key] = true + } + for _, key := range []string{"address", "port"} { + if !keys[key] { + t.Fatalf("http.server ConfigKeys missing %q: %v", key, info.ConfigKeys) + } + } +} + func TestKnownStepTypesPopulated(t *testing.T) { types := KnownStepTypes() if len(types) == 0 { diff --git a/module/http_server.go b/module/http_server.go index b8d1e7b2a..092261194 100644 --- a/module/http_server.go +++ b/module/http_server.go @@ -15,7 +15,7 @@ import ( // HTTPServerConfig holds configuration for the standard HTTP server module. type HTTPServerConfig struct { - Address string `json:"address" yaml:"address" editor:"type=string,required,description=Host:port to listen on (e.g. :8080 or 0.0.0.0:80),default=:8080,placeholder=:8080"` + Address string `json:"address" yaml:"address" editor:"type=string,description=Canonical host:port to listen on (e.g. :8080 or 0.0.0.0:80),default=:8080,placeholder=:8080"` ReadTimeout string `json:"readTimeout" yaml:"readTimeout" editor:"type=duration,description=Maximum duration for reading the full request,default=30s,placeholder=30s"` WriteTimeout string `json:"writeTimeout" yaml:"writeTimeout" editor:"type=duration,description=Maximum duration before timing out writes of the response,default=30s,placeholder=30s"` } diff --git a/module/reflect_validation_test.go b/module/reflect_validation_test.go index 758458f0a..ccc3518bf 100644 --- a/module/reflect_validation_test.go +++ b/module/reflect_validation_test.go @@ -183,7 +183,7 @@ func TestReflectValidation_HTTPServerConfig(t *testing.T) { } address := fields[0] - assertField(t, "address", address, schema.FieldTypeString, true, false) + assertField(t, "address", address, schema.FieldTypeString, false, false) if address.DefaultValue != ":8080" { t.Errorf("address: expected default=:8080, got %v", address.DefaultValue) } diff --git a/plugins/http/modules.go b/plugins/http/modules.go index f81d9c071..79ab78fff 100644 --- a/plugins/http/modules.go +++ b/plugins/http/modules.go @@ -1,6 +1,10 @@ package http import ( + "fmt" + "math" + "strconv" + "strings" "time" "github.com/GoCodeAlone/modular" @@ -35,10 +39,7 @@ func moduleFactories() map[string]plugin.ModuleFactory { } func httpServerFactory(name string, cfg map[string]any) modular.Module { - address := "" - if addr, ok := cfg["address"].(string); ok { - address = addr - } + address := httpServerAddress(cfg) srv := module.NewStandardHTTPServer(name, address) parseDuration := func(key string) time.Duration { @@ -53,6 +54,74 @@ func httpServerFactory(name string, cfg map[string]any) modular.Module { return srv } +func httpServerAddress(cfg map[string]any) string { + if addr, ok := cfg["address"].(string); ok && strings.TrimSpace(addr) != "" { + return addr + } + port, ok := cfg["port"] + if !ok { + return "" + } + if p, ok := normalizeListenPort(port); ok { + return fmt.Sprintf(":%d", p) + } + return "" +} + +func normalizeListenPort(v any) (int, bool) { + switch p := v.(type) { + case int: + return validListenPort(p) + case int8: + return validListenPort(int(p)) + case int16: + return validListenPort(int(p)) + case int32: + return validListenPort(int(p)) + case int64: + return validListenPort(int(p)) + case uint: + return validListenPort(int(p)) + case uint8: + return validListenPort(int(p)) + case uint16: + return validListenPort(int(p)) + case uint32: + return validListenPort(int(p)) + case uint64: + if p > 65535 { + return 0, false + } + return validListenPort(int(p)) + case float64: + if math.Trunc(p) != p || p > 65535 { + return 0, false + } + return validListenPort(int(p)) + case float32: + p64 := float64(p) + if math.Trunc(p64) != p64 || p64 > 65535 { + return 0, false + } + return validListenPort(int(p64)) + case string: + parsed, err := strconv.Atoi(strings.TrimSpace(p)) + if err != nil { + return 0, false + } + return validListenPort(parsed) + default: + return 0, false + } +} + +func validListenPort(port int) (int, bool) { + if port < 0 || port > 65535 { + return 0, false + } + return port, true +} + func httpRouterFactory(name string, _ map[string]any) modular.Module { return module.NewStandardHTTPRouter(name) } diff --git a/plugins/http/plugin_test.go b/plugins/http/plugin_test.go index a366a6ed8..bc1304bad 100644 --- a/plugins/http/plugin_test.go +++ b/plugins/http/plugin_test.go @@ -1,6 +1,7 @@ package http import ( + "reflect" "testing" "github.com/GoCodeAlone/workflow/capability" @@ -22,6 +23,58 @@ func TestNew(t *testing.T) { } } +func TestHTTPServerPortAlias(t *testing.T) { + factory := moduleFactories()["http.server"] + if factory == nil { + t.Fatal("missing http.server factory") + } + + tests := []struct { + name string + cfg map[string]any + want string + }{ + {name: "int port", cfg: map[string]any{"port": 8080}, want: ":8080"}, + {name: "float port", cfg: map[string]any{"port": float64(9090)}, want: ":9090"}, + {name: "string port", cfg: map[string]any{"port": "7070"}, want: ":7070"}, + {name: "address wins", cfg: map[string]any{"address": "127.0.0.1:6060", "port": 7070}, want: "127.0.0.1:6060"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mod := factory("server", tt.cfg) + got := reflect.ValueOf(mod).Elem().FieldByName("address").String() + if got != tt.want { + t.Fatalf("address = %q, want %q", got, tt.want) + } + }) + } +} + +func TestHTTPServerSchemaListsPortAlias(t *testing.T) { + s := httpServerSchema() + fields := make(map[string]schema.ConfigFieldDef, len(s.ConfigFields)) + for _, field := range s.ConfigFields { + fields[field.Key] = field + } + + address, ok := fields["address"] + if !ok { + t.Fatal("http.server schema missing address field") + } + if address.Required { + t.Fatal("address must not be individually required when port is accepted as an alias") + } + + port, ok := fields["port"] + if !ok { + t.Fatal("http.server schema missing port alias field") + } + if port.Type != schema.FieldTypeNumber { + t.Fatalf("port field type = %q, want %q", port.Type, schema.FieldTypeNumber) + } +} + func TestImplementsEnginePlugin(t *testing.T) { var _ plugin.EnginePlugin = New() } diff --git a/plugins/http/schemas.go b/plugins/http/schemas.go index f3c3be9ff..8f30e46b2 100644 --- a/plugins/http/schemas.go +++ b/plugins/http/schemas.go @@ -30,7 +30,8 @@ func httpServerSchema() *schema.ModuleSchema { Description: "Standard HTTP server that listens on a network address", Outputs: []schema.ServiceIODef{{Name: "request", Type: "http.Request", Description: "Incoming HTTP requests"}}, ConfigFields: []schema.ConfigFieldDef{ - {Key: "address", Label: "Listen Address", Type: schema.FieldTypeString, Required: true, Description: "Host:port to listen on (e.g. :8080, 0.0.0.0:80)", DefaultValue: ":8080", Placeholder: ":8080"}, + {Key: "address", Label: "Listen Address", Type: schema.FieldTypeString, Description: "Canonical host:port to listen on (e.g. :8080, 0.0.0.0:80)", DefaultValue: ":8080", Placeholder: ":8080"}, + {Key: "port", Label: "Port Alias", Type: schema.FieldTypeNumber, Description: "Alias for address; normalized to : when address is omitted", Placeholder: "8080"}, }, DefaultConfig: map[string]any{"address": ":8080"}, MaxIncoming: intPtr(0), diff --git a/schema/module_schema.go b/schema/module_schema.go index 4fffdf9db..2cd3fd23c 100644 --- a/schema/module_schema.go +++ b/schema/module_schema.go @@ -125,7 +125,8 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { Description: "Standard HTTP server that listens on a network address", Outputs: []ServiceIODef{{Name: "request", Type: "http.Request", Description: "Incoming HTTP requests"}}, ConfigFields: []ConfigFieldDef{ - {Key: "address", Label: "Listen Address", Type: FieldTypeString, Required: true, Description: "Host:port to listen on (e.g. :8080, 0.0.0.0:80)", DefaultValue: ":8080", Placeholder: ":8080"}, + {Key: "address", Label: "Listen Address", Type: FieldTypeString, Description: "Canonical host:port to listen on (e.g. :8080, 0.0.0.0:80)", DefaultValue: ":8080", Placeholder: ":8080"}, + {Key: "port", Label: "Port Alias", Type: FieldTypeNumber, Description: "Alias for address; normalized to : when address is omitted", Placeholder: "8080"}, }, DefaultConfig: map[string]any{"address": ":8080"}, MaxIncoming: intPtr(0), diff --git a/schema/schema_test.go b/schema/schema_test.go index 9297f7c0b..c76e18528 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -323,7 +323,7 @@ func TestValidateConfig_ValidWorkflowAndTrigger(t *testing.T) { // Module-specific config validation tests // --------------------------------------------------------------------------- -func TestValidateConfig_HTTPServer_MissingAddress(t *testing.T) { +func TestValidateConfig_HTTPServer_MissingAddressOrPort(t *testing.T) { cfg := &config.WorkflowConfig{ Modules: []config.ModuleConfig{ {Name: "srv", Type: "http.server", Config: map[string]any{}}, @@ -331,9 +331,9 @@ func TestValidateConfig_HTTPServer_MissingAddress(t *testing.T) { } err := ValidateConfig(cfg) if err == nil { - t.Fatal("expected error for missing address") + t.Fatal("expected error for missing address or port") } - assertContains(t, err.Error(), `required config field "address" is missing`) + assertContains(t, err.Error(), `requires either "address" or "port"`) } func TestValidateConfig_HTTPServer_NilConfig(t *testing.T) { @@ -346,7 +346,7 @@ func TestValidateConfig_HTTPServer_NilConfig(t *testing.T) { if err == nil { t.Fatal("expected error for nil config") } - assertContains(t, err.Error(), "address") + assertContains(t, err.Error(), `requires either "address" or "port"`) } func TestValidateConfig_HTTPServer_ValidAddress(t *testing.T) { @@ -363,6 +363,20 @@ func TestValidateConfig_HTTPServer_ValidAddress(t *testing.T) { } } +func TestValidateConfig_HTTPServer_ValidPortAlias(t *testing.T) { + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{ + {Name: "srv", Type: "http.server", Config: map[string]any{"port": 9090}}, + }, + Triggers: map[string]any{ + "http": map[string]any{"port": 9090}, + }, + } + if err := ValidateConfig(cfg); err != nil { + t.Errorf("expected valid, got: %v", err) + } +} + func TestValidateConfig_StaticFileserver_MissingRoot(t *testing.T) { cfg := &config.WorkflowConfig{ Modules: []config.ModuleConfig{ diff --git a/schema/validate.go b/schema/validate.go index 88246d4ac..d7c3ea38f 100644 --- a/schema/validate.go +++ b/schema/validate.go @@ -318,6 +318,22 @@ func validateModuleConfig(mod config.ModuleConfig, prefix string, errs *Validati // Additional type-specific structural checks beyond simple required fields switch mod.Type { + case "http.server": + if mod.Config == nil { + *errs = append(*errs, &ValidationError{ + Path: prefix + ".config", + Message: `http.server requires either "address" or "port"`, + }) + break + } + address, _ := mod.Config["address"].(string) + _, hasPort := mod.Config["port"] + if strings.TrimSpace(address) == "" && !hasPort { + *errs = append(*errs, &ValidationError{ + Path: prefix + ".config", + Message: `http.server requires either "address" or "port"`, + }) + } case "database.partitioned": if mod.Config == nil { break From 57fde76f31b58a1b9b81213ac3be593c40f2d2ce Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:23:30 -0400 Subject: [PATCH 07/10] fix: normalize db step config aliases --- cmd/wfctl/modernize_test.go | 94 ++++++++++++++++++ cmd/wfctl/type_registry.go | 6 +- modernize/modernize.go | 1 + modernize/rules.go | 124 ++++++++++++++++++++++++ module/pipeline_step_db_config.go | 55 +++++++++++ module/pipeline_step_db_exec.go | 14 +-- module/pipeline_step_db_exec_test.go | 52 ++++++++++ module/pipeline_step_db_query.go | 14 +-- module/pipeline_step_db_query_cached.go | 14 +-- module/pipeline_step_db_query_test.go | 75 ++++++++++++++ schema/module_schema.go | 17 +++- 11 files changed, 425 insertions(+), 41 deletions(-) create mode 100644 module/pipeline_step_db_config.go diff --git a/cmd/wfctl/modernize_test.go b/cmd/wfctl/modernize_test.go index acaac848c..86583e39b 100644 --- a/cmd/wfctl/modernize_test.go +++ b/cmd/wfctl/modernize_test.go @@ -353,6 +353,100 @@ pipelines: } } +func TestModernizeDBConfigAliases(t *testing.T) { + input := ` +pipelines: + test: + steps: + - name: fetch_user + type: step.db_query + config: + module: my-db + query: "SELECT * FROM users WHERE id = ?" + args: + - u1 + mode: one + - name: save_user + type: step.db_exec + config: + module: my-db + query: "UPDATE users SET name = ? WHERE id = ?" + args: + - Ada + - u1 + mode: many +` + rule := findRule("db-config-aliases") + if rule == nil { + t.Fatal("db-config-aliases rule not found") + } + + doc := parseTestYAML(t, input) + findings := rule.Check(doc, []byte(input)) + if len(findings) < 4 { + t.Fatalf("expected alias findings, got %d: %v", len(findings), findings) + } + changes := rule.Fix(doc) + if len(changes) < 4 { + t.Fatalf("expected alias changes, got %d: %v", len(changes), changes) + } + + out, _ := yaml.Marshal(doc) + result := string(out) + for _, absent := range []string{"module:", "args:", "mode: one", "mode: many"} { + if strings.Contains(result, absent) { + t.Fatalf("expected %q to be modernized, got:\n%s", absent, result) + } + } + for _, present := range []string{"database: my-db", "params:", "mode: single", "mode: list"} { + if !strings.Contains(result, present) { + t.Fatalf("expected %q in modernized output, got:\n%s", present, result) + } + } +} + +func TestModernizeDBConfigAliasesCanonicalWins(t *testing.T) { + input := ` +pipelines: + test: + steps: + - name: fetch_user + type: step.db_query + config: + database: canonical-db + module: alias-db + query: "SELECT * FROM users WHERE id = ?" + params: + - canonical + args: + - alias + mode: one +` + rule := findRule("db-config-aliases") + if rule == nil { + t.Fatal("db-config-aliases rule not found") + } + doc := parseTestYAML(t, input) + changes := rule.Fix(doc) + if len(changes) == 0 { + t.Fatal("expected alias cleanup changes") + } + + out, _ := yaml.Marshal(doc) + result := string(out) + for _, absent := range []string{"module:", "args:"} { + if strings.Contains(result, absent) { + t.Fatalf("expected %q to be removed, got:\n%s", absent, result) + } + } + if strings.Contains(result, "alias-db") || strings.Contains(result, "alias") { + t.Fatalf("expected canonical fields to win, got:\n%s", result) + } + if !strings.Contains(result, "database: canonical-db") || !strings.Contains(result, "- canonical") { + t.Fatalf("expected canonical fields to remain, got:\n%s", result) + } +} + func TestDbQueryIndexCheck(t *testing.T) { input := ` pipelines: diff --git a/cmd/wfctl/type_registry.go b/cmd/wfctl/type_registry.go index 6a0e2b2a5..ca4ac4a50 100644 --- a/cmd/wfctl/type_registry.go +++ b/cmd/wfctl/type_registry.go @@ -777,17 +777,17 @@ func KnownStepTypes() map[string]StepTypeInfo { "step.db_query": { Type: "step.db_query", Plugin: "pipelinesteps", - ConfigKeys: []string{"database", "query", "params", "tenantKey"}, + ConfigKeys: []string{"database", "module", "query", "params", "args", "mode", "tenantKey"}, }, "step.db_exec": { Type: "step.db_exec", Plugin: "pipelinesteps", - ConfigKeys: []string{"database", "query", "params", "tenantKey"}, + ConfigKeys: []string{"database", "module", "query", "params", "args", "mode", "tenantKey"}, }, "step.db_query_cached": { Type: "step.db_query_cached", Plugin: "pipelinesteps", - ConfigKeys: []string{"database", "query", "params", "cache_key", "cache_ttl", "scan_fields"}, + ConfigKeys: []string{"database", "module", "query", "params", "args", "mode", "cache_key", "cache_ttl", "scan_fields"}, }, "step.db_create_partition": { Type: "step.db_create_partition", diff --git a/modernize/modernize.go b/modernize/modernize.go index 0edf0b244..3ee22595b 100644 --- a/modernize/modernize.go +++ b/modernize/modernize.go @@ -37,6 +37,7 @@ func AllRules() []Rule { return []Rule{ hyphenStepsRule(), conditionalFieldRule(), + dbConfigAliasesRule(), dbQueryModeRule(), dbQueryIndexRule(), absoluteDbPathRule(), diff --git a/modernize/rules.go b/modernize/rules.go index 4ce443b69..324ab80a5 100644 --- a/modernize/rules.go +++ b/modernize/rules.go @@ -35,6 +35,25 @@ func findMapValue(node *yaml.Node, key string) *yaml.Node { return nil } +func findMapKeyValueIndex(node *yaml.Node, key string) (int, *yaml.Node, *yaml.Node) { + if node.Kind != yaml.MappingNode { + return -1, nil, nil + } + for i := 0; i+1 < len(node.Content); i += 2 { + if node.Content[i].Value == key { + return i, node.Content[i], node.Content[i+1] + } + } + return -1, nil, nil +} + +func removeMapKeyValue(node *yaml.Node, idx int) { + if node == nil || node.Kind != yaml.MappingNode || idx < 0 || idx+1 >= len(node.Content) { + return + } + node.Content = append(node.Content[:idx], node.Content[idx+2:]...) +} + // stepNameInfo holds a step name and its YAML line number. type stepNameInfo struct { Name string @@ -434,6 +453,111 @@ func dbQueryModeRule() Rule { } } +func dbConfigAliasesRule() Rule { + stepTypes := []string{"step.db_query", "step.db_exec", "step.db_query_cached"} + return Rule{ + ID: "db-config-aliases", + Description: "Rewrite DB step aliases to canonical database/params/list/single config", + Severity: "warning", + Check: func(root *yaml.Node, raw []byte) []Finding { + var findings []Finding + for _, stepType := range stepTypes { + forEachStepOfType(root, stepType, func(step *yaml.Node) { + cfg := findMapValue(step, "config") + if cfg == nil || cfg.Kind != yaml.MappingNode { + return + } + if _, key, _ := findMapKeyValueIndex(cfg, "module"); key != nil { + findings = append(findings, Finding{ + RuleID: "db-config-aliases", + Line: key.Line, + Message: fmt.Sprintf("%s uses alias config key \"module\"; use \"database\"", stepType), + Fixable: true, + }) + } + if _, key, _ := findMapKeyValueIndex(cfg, "args"); key != nil { + findings = append(findings, Finding{ + RuleID: "db-config-aliases", + Line: key.Line, + Message: fmt.Sprintf("%s uses alias config key \"args\"; use \"params\"", stepType), + Fixable: true, + }) + } + if _, _, mode := findMapKeyValueIndex(cfg, "mode"); mode != nil && mode.Kind == yaml.ScalarNode { + switch mode.Value { + case "one", "many": + findings = append(findings, Finding{ + RuleID: "db-config-aliases", + Line: mode.Line, + Message: fmt.Sprintf("%s uses mode %q; use %q", stepType, mode.Value, modernDBMode(mode.Value)), + Fixable: true, + }) + } + } + }) + } + return findings + }, + Fix: func(root *yaml.Node) []Change { + var changes []Change + for _, stepType := range stepTypes { + forEachStepOfType(root, stepType, func(step *yaml.Node) { + cfg := findMapValue(step, "config") + if cfg == nil || cfg.Kind != yaml.MappingNode { + return + } + modernizeConfigKeyAlias(cfg, "module", "database", "db-config-aliases", stepType, &changes) + modernizeConfigKeyAlias(cfg, "args", "params", "db-config-aliases", stepType, &changes) + if _, _, mode := findMapKeyValueIndex(cfg, "mode"); mode != nil && mode.Kind == yaml.ScalarNode { + if modern := modernDBMode(mode.Value); modern != mode.Value { + changes = append(changes, Change{ + RuleID: "db-config-aliases", + Line: mode.Line, + Description: fmt.Sprintf("Updated %s mode %q -> %q", stepType, mode.Value, modern), + }) + mode.Value = modern + } + } + }) + } + return changes + }, + } +} + +func modernDBMode(mode string) string { + switch mode { + case "one": + return "single" + case "many": + return "list" + default: + return mode + } +} + +func modernizeConfigKeyAlias(cfg *yaml.Node, alias, canonical, ruleID, stepType string, changes *[]Change) { + aliasIdx, aliasKey, _ := findMapKeyValueIndex(cfg, alias) + if aliasKey == nil { + return + } + if _, canonicalKey, _ := findMapKeyValueIndex(cfg, canonical); canonicalKey != nil { + *changes = append(*changes, Change{ + RuleID: ruleID, + Line: aliasKey.Line, + Description: fmt.Sprintf("Removed %s alias %q because canonical %q is present", stepType, alias, canonical), + }) + removeMapKeyValue(cfg, aliasIdx) + return + } + *changes = append(*changes, Change{ + RuleID: ruleID, + Line: aliasKey.Line, + Description: fmt.Sprintf("Renamed %s config key %q -> %q", stepType, alias, canonical), + }) + aliasKey.Value = canonical +} + // dotRowAccessRegex matches patterns like .steps.stepname.row.column inside {{ }}. var dotRowAccessRegex = regexp.MustCompile(`\.steps\.(\w+)\.row\.(\w+)`) diff --git a/module/pipeline_step_db_config.go b/module/pipeline_step_db_config.go new file mode 100644 index 000000000..5ec0df6ff --- /dev/null +++ b/module/pipeline_step_db_config.go @@ -0,0 +1,55 @@ +package module + +func configStringAlias(cfg map[string]any, canonical string, aliases ...string) string { + if v, ok := cfg[canonical].(string); ok && v != "" { + return v + } + for _, alias := range aliases { + if v, ok := cfg[alias].(string); ok && v != "" { + return v + } + } + return "" +} + +func configStringListAlias(cfg map[string]any, canonical string, aliases ...string) []string { + if values, ok := configStringList(cfg[canonical]); ok { + return values + } + for _, alias := range aliases { + if values, ok := configStringList(cfg[alias]); ok { + return values + } + } + return nil +} + +func configStringList(v any) ([]string, bool) { + switch list := v.(type) { + case []any: + values := make([]string, 0, len(list)) + for _, item := range list { + if s, ok := item.(string); ok { + values = append(values, s) + } + } + return values, true + case []string: + values := make([]string, len(list)) + copy(values, list) + return values, true + default: + return nil, false + } +} + +func normalizeDBMode(mode string) string { + switch mode { + case "many": + return "list" + case "one": + return "single" + default: + return mode + } +} diff --git a/module/pipeline_step_db_exec.go b/module/pipeline_step_db_exec.go index c44b043ba..9d2bc1d39 100644 --- a/module/pipeline_step_db_exec.go +++ b/module/pipeline_step_db_exec.go @@ -26,7 +26,7 @@ type DBExecStep struct { // NewDBExecStepFactory returns a StepFactory that creates DBExecStep instances. func NewDBExecStepFactory() StepFactory { return func(name string, config map[string]any, app modular.Application) (PipelineStep, error) { - database, _ := config["database"].(string) + database := configStringAlias(config, "database", "module") if database == "" { return nil, fmt.Errorf("db_exec step %q: 'database' is required", name) } @@ -43,22 +43,14 @@ func NewDBExecStepFactory() StepFactory { return nil, fmt.Errorf("db_exec step %q: query must not contain template expressions (use params instead)", name) } - var params []string - if p, ok := config["params"]; ok { - if list, ok := p.([]any); ok { - for _, item := range list { - if s, ok := item.(string); ok { - params = append(params, s) - } - } - } - } + params := configStringListAlias(config, "params", "args") ignoreError, _ := config["ignore_error"].(bool) tenantKey, _ := config["tenantKey"].(string) returning, _ := config["returning"].(bool) mode, _ := config["mode"].(string) + mode = normalizeDBMode(mode) if returning { if mode == "" { mode = "list" diff --git a/module/pipeline_step_db_exec_test.go b/module/pipeline_step_db_exec_test.go index 18542bae7..a4083e55c 100644 --- a/module/pipeline_step_db_exec_test.go +++ b/module/pipeline_step_db_exec_test.go @@ -9,6 +9,58 @@ import ( _ "modernc.org/sqlite" ) +func TestDBExecStep_AliasNormalization(t *testing.T) { + factory := NewDBExecStepFactory() + step, err := factory("update", map[string]any{ + "module": "alias-db", + "query": "UPDATE items SET name = ? WHERE id = ?", + "args": []any{"Widget", "i1"}, + "returning": true, + "mode": "one", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + dbStep := step.(*DBExecStep) + if dbStep.database != "alias-db" { + t.Fatalf("database = %q, want alias-db", dbStep.database) + } + if dbStep.mode != "single" { + t.Fatalf("mode = %q, want single", dbStep.mode) + } + if got := strings.Join(dbStep.params, ","); got != "Widget,i1" { + t.Fatalf("params = %q, want Widget,i1", got) + } +} + +func TestDBExecStep_CanonicalConfigWinsOverAliases(t *testing.T) { + factory := NewDBExecStepFactory() + step, err := factory("update", map[string]any{ + "database": "canonical-db", + "module": "alias-db", + "query": "UPDATE items SET name = ? WHERE id = ?", + "params": []any{"Canonical", "i1"}, + "args": []any{"Alias", "i1"}, + "returning": true, + "mode": "many", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + dbStep := step.(*DBExecStep) + if dbStep.database != "canonical-db" { + t.Fatalf("database = %q, want canonical-db", dbStep.database) + } + if dbStep.mode != "list" { + t.Fatalf("mode = %q, want list", dbStep.mode) + } + if got := strings.Join(dbStep.params, ","); got != "Canonical,i1" { + t.Fatalf("params = %q, want Canonical,i1", got) + } +} + func TestDBExecStep_Insert(t *testing.T) { db, err := sql.Open("sqlite", ":memory:") if err != nil { diff --git a/module/pipeline_step_db_query.go b/module/pipeline_step_db_query.go index 859f54a31..540f211e0 100644 --- a/module/pipeline_step_db_query.go +++ b/module/pipeline_step_db_query.go @@ -39,7 +39,7 @@ type DBQueryStep struct { // NewDBQueryStepFactory returns a StepFactory that creates DBQueryStep instances. func NewDBQueryStepFactory() StepFactory { return func(name string, config map[string]any, app modular.Application) (PipelineStep, error) { - database, _ := config["database"].(string) + database := configStringAlias(config, "database", "module") if database == "" { return nil, fmt.Errorf("db_query step %q: 'database' is required", name) } @@ -56,18 +56,10 @@ func NewDBQueryStepFactory() StepFactory { return nil, fmt.Errorf("db_query step %q: query must not contain template expressions (use params instead)", name) } - var params []string - if p, ok := config["params"]; ok { - if list, ok := p.([]any); ok { - for _, item := range list { - if s, ok := item.(string); ok { - params = append(params, s) - } - } - } - } + params := configStringListAlias(config, "params", "args") mode, _ := config["mode"].(string) + mode = normalizeDBMode(mode) if mode == "" { mode = "list" } diff --git a/module/pipeline_step_db_query_cached.go b/module/pipeline_step_db_query_cached.go index fe1fad911..f4ca09263 100644 --- a/module/pipeline_step_db_query_cached.go +++ b/module/pipeline_step_db_query_cached.go @@ -38,7 +38,7 @@ type DBQueryCachedStep struct { // NewDBQueryCachedStepFactory returns a StepFactory that creates DBQueryCachedStep instances. func NewDBQueryCachedStepFactory() StepFactory { return func(name string, config map[string]any, app modular.Application) (PipelineStep, error) { - database, _ := config["database"].(string) + database := configStringAlias(config, "database", "module") if database == "" { return nil, fmt.Errorf("db_query_cached step %q: 'database' is required", name) } @@ -70,16 +70,7 @@ func NewDBQueryCachedStepFactory() StepFactory { cacheTTL = parsed } - var params []string - if p, ok := config["params"]; ok { - if list, ok := p.([]any); ok { - for _, item := range list { - if s, ok := item.(string); ok { - params = append(params, s) - } - } - } - } + params := configStringListAlias(config, "params", "args") var scanFields []string if sf, ok := config["scan_fields"]; ok { @@ -93,6 +84,7 @@ func NewDBQueryCachedStepFactory() StepFactory { } mode, _ := config["mode"].(string) + mode = normalizeDBMode(mode) if mode == "" { mode = "single" } diff --git a/module/pipeline_step_db_query_test.go b/module/pipeline_step_db_query_test.go index 21b43f34b..24fd42d83 100644 --- a/module/pipeline_step_db_query_test.go +++ b/module/pipeline_step_db_query_test.go @@ -72,6 +72,81 @@ func mockAppWithDB(name string, db *sql.DB) *MockApplication { return app } +func TestDBQueryStep_AliasNormalization(t *testing.T) { + factory := NewDBQueryStepFactory() + step, err := factory("fetch", map[string]any{ + "module": "alias-db", + "query": "SELECT id FROM companies WHERE id = ?", + "args": []any{"c1"}, + "mode": "many", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + dbStep := step.(*DBQueryStep) + if dbStep.database != "alias-db" { + t.Fatalf("database = %q, want alias-db", dbStep.database) + } + if dbStep.mode != "list" { + t.Fatalf("mode = %q, want list", dbStep.mode) + } + if got := strings.Join(dbStep.params, ","); got != "c1" { + t.Fatalf("params = %q, want c1", got) + } +} + +func TestDBQueryStep_CanonicalConfigWinsOverAliases(t *testing.T) { + factory := NewDBQueryStepFactory() + step, err := factory("fetch", map[string]any{ + "database": "canonical-db", + "module": "alias-db", + "query": "SELECT id FROM companies WHERE id = ?", + "params": []any{"canonical"}, + "args": []any{"alias"}, + "mode": "one", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + dbStep := step.(*DBQueryStep) + if dbStep.database != "canonical-db" { + t.Fatalf("database = %q, want canonical-db", dbStep.database) + } + if dbStep.mode != "single" { + t.Fatalf("mode = %q, want single", dbStep.mode) + } + if got := strings.Join(dbStep.params, ","); got != "canonical" { + t.Fatalf("params = %q, want canonical", got) + } +} + +func TestDBQueryCachedStep_AliasNormalization(t *testing.T) { + factory := NewDBQueryCachedStepFactory() + step, err := factory("fetch-cached", map[string]any{ + "module": "alias-db", + "query": "SELECT id FROM companies WHERE id = ?", + "args": []any{"c1"}, + "cache_key": "company:c1", + "mode": "many", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + dbStep := step.(*DBQueryCachedStep) + if dbStep.database != "alias-db" { + t.Fatalf("database = %q, want alias-db", dbStep.database) + } + if dbStep.mode != "list" { + t.Fatalf("mode = %q, want list", dbStep.mode) + } + if got := strings.Join(dbStep.params, ","); got != "c1" { + t.Fatalf("params = %q, want c1", got) + } +} + func TestDBQueryStep_ListMode(t *testing.T) { db := setupTestDB(t) app := mockAppWithDB("test-db", db) diff --git a/schema/module_schema.go b/schema/module_schema.go index 2cd3fd23c..5eafffc32 100644 --- a/schema/module_schema.go +++ b/schema/module_schema.go @@ -1039,10 +1039,12 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { Inputs: []ServiceIODef{{Name: "context", Type: "PipelineContext", Description: "Pipeline context for template parameter resolution"}}, Outputs: []ServiceIODef{{Name: "result", Type: "StepResult", Description: "Query results as rows/count (list mode) or row/found (single mode)"}}, ConfigFields: []ConfigFieldDef{ - {Key: "database", Label: "Database", Type: FieldTypeString, Required: true, Description: "Name of the database service (must implement DBProvider)", Placeholder: "admin-db", InheritFrom: "dependency.name"}, + {Key: "database", Label: "Database", Type: FieldTypeString, Description: "Canonical name of the database service (must implement DBProvider)", Placeholder: "admin-db", InheritFrom: "dependency.name"}, + {Key: "module", Label: "Database Alias", Type: FieldTypeString, Description: "Alias for database; wfctl modernize rewrites this to database", Placeholder: "admin-db", InheritFrom: "dependency.name"}, {Key: "query", Label: "SQL Query", Type: FieldTypeSQL, Required: true, Description: "Parameterized SQL SELECT query (use ? for placeholders). Template expressions are forbidden unless allow_dynamic_sql is true.", Placeholder: "SELECT id, name FROM companies WHERE id = ?"}, {Key: "params", Label: "Parameters", Type: FieldTypeArray, ArrayItemType: "string", Description: "Template-resolved parameter values for ? placeholders in query"}, - {Key: "mode", Label: "Mode", Type: FieldTypeSelect, Options: []string{"list", "single"}, DefaultValue: "list", Description: "Result mode: 'list' returns rows/count, 'single' returns row/found"}, + {Key: "args", Label: "Parameters Alias", Type: FieldTypeArray, ArrayItemType: "string", Description: "Alias for params; wfctl modernize rewrites this to params"}, + {Key: "mode", Label: "Mode", Type: FieldTypeSelect, Options: []string{"list", "single", "many", "one"}, DefaultValue: "list", Description: "Result mode: list/many returns rows/count, single/one returns row/found"}, {Key: "tenantKey", Label: "Tenant Key", Type: FieldTypeString, Description: "Dot-path in pipeline context to resolve the tenant value for automatic scoping (requires database.partitioned)", Placeholder: "steps.auth.tenant_id"}, {Key: "allow_dynamic_sql", Label: "Allow Dynamic SQL", Type: FieldTypeBool, DefaultValue: "false", Description: "When true, template expressions in 'query' are resolved at runtime. Each resolved value must contain only letters, digits, underscores and hyphens to prevent SQL injection."}, }, @@ -1056,10 +1058,12 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { Inputs: []ServiceIODef{{Name: "context", Type: "PipelineContext", Description: "Pipeline context for template parameter and cache key resolution"}}, Outputs: []ServiceIODef{{Name: "result", Type: "StepResult", Description: "Query results as rows/count (list mode) or row/found (single mode), plus cache_hit boolean"}}, ConfigFields: []ConfigFieldDef{ - {Key: "database", Label: "Database", Type: FieldTypeString, Required: true, Description: "Name of the database service (must implement DBProvider)", Placeholder: "db", InheritFrom: "dependency.name"}, + {Key: "database", Label: "Database", Type: FieldTypeString, Description: "Canonical name of the database service (must implement DBProvider)", Placeholder: "db", InheritFrom: "dependency.name"}, + {Key: "module", Label: "Database Alias", Type: FieldTypeString, Description: "Alias for database; wfctl modernize rewrites this to database", Placeholder: "db", InheritFrom: "dependency.name"}, {Key: "query", Label: "SQL Query", Type: FieldTypeSQL, Required: true, Description: "Parameterized SQL SELECT query using $N placeholders (e.g. $1, $2); automatically converted to ? for SQLite drivers. Template expressions are forbidden unless allow_dynamic_sql is true.", Placeholder: "SELECT backend_url, settings FROM routing_config WHERE tenant_id = $1 LIMIT 1"}, {Key: "params", Label: "Parameters", Type: FieldTypeArray, ArrayItemType: "string", Description: "Template-resolved parameter values for query placeholders"}, - {Key: "mode", Label: "Mode", Type: FieldTypeSelect, Options: []string{"single", "list"}, DefaultValue: "single", Description: "Result mode: 'single' returns row/found, 'list' returns rows/count"}, + {Key: "args", Label: "Parameters Alias", Type: FieldTypeArray, ArrayItemType: "string", Description: "Alias for params; wfctl modernize rewrites this to params"}, + {Key: "mode", Label: "Mode", Type: FieldTypeSelect, Options: []string{"single", "list", "one", "many"}, DefaultValue: "single", Description: "Result mode: single/one returns row/found, list/many returns rows/count"}, {Key: "cache_key", Label: "Cache Key", Type: FieldTypeString, Required: true, Description: "Template-resolved key used to store/retrieve the cached result", Placeholder: "tenant_config:{{.steps.parse.headers.X-Tenant-Id}}"}, {Key: "cache_ttl", Label: "Cache TTL", Type: FieldTypeString, DefaultValue: "5m", Description: "Duration string for how long to cache the result (e.g. '5m', '30s', '1h')", Placeholder: "5m"}, {Key: "scan_fields", Label: "Scan Fields", Type: FieldTypeArray, ArrayItemType: "string", Description: "Column names to include in the output (omit to include all columns)"}, @@ -1075,9 +1079,12 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { Inputs: []ServiceIODef{{Name: "context", Type: "PipelineContext", Description: "Pipeline context for template parameter resolution"}}, Outputs: []ServiceIODef{{Name: "result", Type: "StepResult", Description: "Execution result with affected_rows and last_id"}}, ConfigFields: []ConfigFieldDef{ - {Key: "database", Label: "Database", Type: FieldTypeString, Required: true, Description: "Name of the database service (must implement DBProvider)", Placeholder: "admin-db", InheritFrom: "dependency.name"}, + {Key: "database", Label: "Database", Type: FieldTypeString, Description: "Canonical name of the database service (must implement DBProvider)", Placeholder: "admin-db", InheritFrom: "dependency.name"}, + {Key: "module", Label: "Database Alias", Type: FieldTypeString, Description: "Alias for database; wfctl modernize rewrites this to database", Placeholder: "admin-db", InheritFrom: "dependency.name"}, {Key: "query", Label: "SQL Statement", Type: FieldTypeSQL, Required: true, Description: "Parameterized SQL INSERT/UPDATE/DELETE statement (use ? for placeholders). Template expressions are forbidden unless allow_dynamic_sql is true.", Placeholder: "INSERT INTO companies (id, name) VALUES (?, ?)"}, {Key: "params", Label: "Parameters", Type: FieldTypeArray, ArrayItemType: "string", Description: "Template-resolved parameter values for ? placeholders"}, + {Key: "args", Label: "Parameters Alias", Type: FieldTypeArray, ArrayItemType: "string", Description: "Alias for params; wfctl modernize rewrites this to params"}, + {Key: "mode", Label: "Mode", Type: FieldTypeSelect, Options: []string{"list", "single", "many", "one"}, Description: "Result mode for returning statements: list/many returns rows/count, single/one returns row/found"}, {Key: "tenantKey", Label: "Tenant Key", Type: FieldTypeString, Description: "Dot-path in pipeline context to resolve the tenant value for automatic scoping. Supported for UPDATE/DELETE only (requires database.partitioned)", Placeholder: "steps.auth.tenant_id"}, {Key: "allow_dynamic_sql", Label: "Allow Dynamic SQL", Type: FieldTypeBool, DefaultValue: "false", Description: "When true, template expressions in 'query' are resolved at runtime. Each resolved value must contain only letters, digits, underscores and hyphens to prevent SQL injection."}, }, From 72d8b53abf6f5771add3e1f68ce6f9b0ab839c6e Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:28:53 -0400 Subject: [PATCH 08/10] fix: add pipeline authoring aliases --- cmd/wfctl/type_registry.go | 9 ++- cmd/wfctl/type_registry_test.go | 1 + handlers/testhelpers_test.go | 1 + module/pipeline_step_conditional.go | 71 ++++++++++++++++++++++ module/pipeline_step_conditional_test.go | 54 ++++++++++++++++ module/pipeline_step_json_response.go | 20 +++++- module/pipeline_step_json_response_test.go | 64 +++++++++++++++++++ module/pipeline_step_request_parse.go | 3 + module/pipeline_step_request_parse_test.go | 31 ++++++++++ plugins/pipelinesteps/plugin.go | 3 + plugins/pipelinesteps/plugin_test.go | 34 +++++++++++ schema/module_schema.go | 27 +++++++- 12 files changed, 311 insertions(+), 7 deletions(-) diff --git a/cmd/wfctl/type_registry.go b/cmd/wfctl/type_registry.go index ca4ac4a50..d50cc7c09 100644 --- a/cmd/wfctl/type_registry.go +++ b/cmd/wfctl/type_registry.go @@ -712,7 +712,7 @@ func KnownStepTypes() map[string]StepTypeInfo { "step.conditional": { Type: "step.conditional", Plugin: "pipelinesteps", - ConfigKeys: []string{"condition", "then", "else"}, + ConfigKeys: []string{"field", "routes", "default", "if", "then", "else"}, }, "step.set": { Type: "step.set", @@ -772,7 +772,7 @@ func KnownStepTypes() map[string]StepTypeInfo { "step.request_parse": { Type: "step.request_parse", Plugin: "pipelinesteps", - ConfigKeys: []string{"body", "query", "headers"}, + ConfigKeys: []string{"body", "query", "headers", "path_params", "query_params", "parse_body", "parse_headers", "format"}, }, "step.db_query": { Type: "step.db_query", @@ -804,6 +804,11 @@ func KnownStepTypes() map[string]StepTypeInfo { Plugin: "pipelinesteps", ConfigKeys: []string{"status", "body", "headers"}, }, + "step.response": { + Type: "step.response", + Plugin: "pipelinesteps", + ConfigKeys: []string{"status", "status_from", "body", "body_from", "headers"}, + }, "step.static_file": { Type: "step.static_file", Plugin: "pipelinesteps", diff --git a/cmd/wfctl/type_registry_test.go b/cmd/wfctl/type_registry_test.go index 86c1ff213..d3486a2f1 100644 --- a/cmd/wfctl/type_registry_test.go +++ b/cmd/wfctl/type_registry_test.go @@ -110,6 +110,7 @@ func TestKnownStepTypesPopulated(t *testing.T) { "step.db_query", "step.publish", "step.http_call", + "step.response", "step.cache_get", "step.auth_validate", "step.authz_check", diff --git a/handlers/testhelpers_test.go b/handlers/testhelpers_test.go index 46bdbcbb8..72d4cda1e 100644 --- a/handlers/testhelpers_test.go +++ b/handlers/testhelpers_test.go @@ -282,6 +282,7 @@ func registerCyclicPluginFactories(engine *workflow.StdEngine) { engine.AddStepType("step.db_query", module.NewDBQueryStepFactory()) engine.AddStepType("step.db_exec", module.NewDBExecStepFactory()) engine.AddStepType("step.json_response", module.NewJSONResponseStepFactory()) + engine.AddStepType("step.response", module.NewJSONResponseStepFactory()) // Workflow handlers from the cyclic plugins are registered directly by // individual tests via engine.RegisterWorkflowHandler(), so we don't diff --git a/module/pipeline_step_conditional.go b/module/pipeline_step_conditional.go index af1bfe16c..206c7a45a 100644 --- a/module/pipeline_step_conditional.go +++ b/module/pipeline_step_conditional.go @@ -3,9 +3,11 @@ package module import ( "context" "fmt" + "regexp" "strings" "github.com/GoCodeAlone/modular" + "github.com/GoCodeAlone/workflow/pipeline" ) // ConditionalStep routes pipeline execution to different steps based on a @@ -15,12 +17,33 @@ type ConditionalStep struct { field string routes map[string]string defaultRoute string + ifExpr string + thenStep string + elseStep string tmpl *TemplateEngine } +var leadingDotExprIdentRe = regexp.MustCompile(`(^|[\s(=!<>])\.([A-Za-z_][A-Za-z0-9_]*)`) + // NewConditionalStepFactory returns a StepFactory that creates ConditionalStep instances. func NewConditionalStepFactory() StepFactory { return func(name string, config map[string]any, _ modular.Application) (PipelineStep, error) { + ifExpr, _ := config["if"].(string) + if ifExpr != "" { + thenStep, _ := config["then"].(string) + if thenStep == "" { + return nil, fmt.Errorf("conditional step %q: 'then' is required when 'if' is configured", name) + } + elseStep, _ := config["else"].(string) + return &ConditionalStep{ + name: name, + ifExpr: ifExpr, + thenStep: thenStep, + elseStep: elseStep, + tmpl: NewTemplateEngine(), + }, nil + } + field, _ := config["field"].(string) if field == "" { return nil, fmt.Errorf("conditional step %q: 'field' is required", name) @@ -57,6 +80,25 @@ func (s *ConditionalStep) Name() string { return s.name } // Execute resolves the field value and determines the next step. func (s *ConditionalStep) Execute(_ context.Context, pc *PipelineContext) (*StepResult, error) { + if s.ifExpr != "" { + condition, err := s.evaluateIf(pc) + if err != nil { + return nil, fmt.Errorf("conditional step %q: failed to evaluate if expression: %w", s.name, err) + } + nextStep := s.elseStep + if condition { + nextStep = s.thenStep + } + output := map[string]any{"condition": condition} + if nextStep != "" { + output["next_step"] = nextStep + } + return &StepResult{ + Output: output, + NextStep: nextStep, + }, nil + } + // Resolve the field value. If the field already contains template // delimiters ({{ }}), use it as-is. Otherwise, build a template // expression from the dot-separated field path. @@ -90,6 +132,35 @@ func (s *ConditionalStep) Execute(_ context.Context, pc *PipelineContext) (*Step return nil, fmt.Errorf("conditional step %q: value %q not found in routes and no default configured", s.name, resolved) } +func (s *ConditionalStep) evaluateIf(pc *PipelineContext) (bool, error) { + expr := strings.TrimSpace(s.ifExpr) + if strings.HasPrefix(expr, "{{") && strings.HasSuffix(expr, "}}") { + inner := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(expr, "{{"), "}}")) + if strings.Contains(inner, "==") || strings.Contains(inner, "!=") || strings.Contains(inner, ">") || strings.Contains(inner, "<") { + normalized := leadingDotExprIdentRe.ReplaceAllString(inner, `${1}${2}`) + resolved, err := pipeline.NewExprEngine().Evaluate(normalized, pc) + if err != nil { + return false, err + } + return truthyString(resolved), nil + } + } + resolved, err := s.tmpl.Resolve(expr, pc) + if err != nil { + return false, err + } + return truthyString(resolved), nil +} + +func truthyString(value string) bool { + switch strings.ToLower(strings.TrimSpace(value)) { + case "", "0", "false", "no", "n", "off", "": + return false + default: + return true + } +} + // buildFieldTemplate converts a dot-separated field path into a Go template // expression. If any segment contains characters that are invalid in Go // template identifiers (like hyphens), the entire path is built using diff --git a/module/pipeline_step_conditional_test.go b/module/pipeline_step_conditional_test.go index d858407d0..ca3ac3f04 100644 --- a/module/pipeline_step_conditional_test.go +++ b/module/pipeline_step_conditional_test.go @@ -81,6 +81,60 @@ func TestConditionalStep_UsesDefaultWhenNoMatch(t *testing.T) { } } +func TestConditionalStep_IfThenElse(t *testing.T) { + factory := NewConditionalStepFactory() + step, err := factory("route-active", map[string]any{ + "if": `{{ .status == "active" }}`, + "then": "proceed", + "else": "reject", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + active := NewPipelineContext(map[string]any{"status": "active"}, nil) + result, err := step.Execute(context.Background(), active) + if err != nil { + t.Fatalf("execute active: %v", err) + } + if result.NextStep != "proceed" { + t.Fatalf("active NextStep = %q, want proceed", result.NextStep) + } + if result.Output["condition"] != true { + t.Fatalf("condition output = %v, want true", result.Output["condition"]) + } + + inactive := NewPipelineContext(map[string]any{"status": "inactive"}, nil) + result, err = step.Execute(context.Background(), inactive) + if err != nil { + t.Fatalf("execute inactive: %v", err) + } + if result.NextStep != "reject" { + t.Fatalf("inactive NextStep = %q, want reject", result.NextStep) + } +} + +func TestConditionalStep_IfThenElseExpr(t *testing.T) { + factory := NewConditionalStepFactory() + step, err := factory("route-active", map[string]any{ + "if": `${ status == "active" }`, + "then": "proceed", + "else": "reject", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := NewPipelineContext(map[string]any{"status": "active"}, nil) + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute: %v", err) + } + if result.NextStep != "proceed" { + t.Fatalf("NextStep = %q, want proceed", result.NextStep) + } +} + func TestConditionalStep_ErrorWhenNoMatchAndNoDefault(t *testing.T) { factory := NewConditionalStepFactory() step, err := factory("route-strict", map[string]any{ diff --git a/module/pipeline_step_json_response.go b/module/pipeline_step_json_response.go index 048abd212..299eb9866 100644 --- a/module/pipeline_step_json_response.go +++ b/module/pipeline_step_json_response.go @@ -178,7 +178,7 @@ func (s *JSONResponseStep) resolveResponseBody(pc *PipelineContext) any { if err != nil { return s.bodyRaw } - return resolved + return decodeJSONObjectOrArray(resolved) } return s.bodyRaw } @@ -228,12 +228,28 @@ func (s *JSONResponseStep) resolveBodyValue(v any, pc *PipelineContext) (any, er } return result, nil case string: - return s.tmpl.Resolve(val, pc) + resolved, err := s.tmpl.Resolve(val, pc) + if err != nil { + return nil, err + } + return decodeJSONObjectOrArray(resolved), nil default: return v, nil } } +func decodeJSONObjectOrArray(value string) any { + trimmed := strings.TrimSpace(value) + if !strings.HasPrefix(trimmed, "{") && !strings.HasPrefix(trimmed, "[") { + return value + } + var decoded any + if err := json.Unmarshal([]byte(trimmed), &decoded); err != nil { + return value + } + return decoded +} + // resolveBodyFrom resolves a dotted path like "steps.get-company.row" from the // pipeline context. It looks in StepOutputs first (for "steps.X.Y" paths), // then in Current. diff --git a/module/pipeline_step_json_response_test.go b/module/pipeline_step_json_response_test.go index 99b6eb024..e950b77ab 100644 --- a/module/pipeline_step_json_response_test.go +++ b/module/pipeline_step_json_response_test.go @@ -180,6 +180,70 @@ func TestJSONResponseStep_TemplateBody(t *testing.T) { } } +func TestJSONResponseStep_TemplateRawJSONArray(t *testing.T) { + factory := NewJSONResponseStepFactory() + step, err := factory("templated-array", map[string]any{ + "status": 200, + "body": "{{ .steps.list.json }}", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + recorder := httptest.NewRecorder() + pc := NewPipelineContext(nil, map[string]any{ + "_http_response_writer": recorder, + }) + pc.MergeStepOutput("list", map[string]any{ + "json": `[{"id":"c1"},{"id":"c2"}]`, + }) + + _, err = step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute error: %v", err) + } + + var body []map[string]any + if err := json.NewDecoder(recorder.Body).Decode(&body); err != nil { + t.Fatalf("decode response: %v", err) + } + if len(body) != 2 { + t.Fatalf("expected 2 items, got %d", len(body)) + } + if body[0]["id"] != "c1" { + t.Fatalf("expected first id c1, got %v", body[0]["id"]) + } +} + +func TestJSONResponseStep_TemplateRawJSONObject(t *testing.T) { + factory := NewJSONResponseStepFactory() + step, err := factory("templated-object", map[string]any{ + "status": 200, + "body": "{{ .steps.fetch.json }}", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := NewPipelineContext(nil, nil) + pc.MergeStepOutput("fetch", map[string]any{ + "json": `{"id":"c1","name":"Acme"}`, + }) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute error: %v", err) + } + + body, ok := result.Output["body"].(map[string]any) + if !ok { + t.Fatalf("expected map body, got %T", result.Output["body"]) + } + if body["id"] != "c1" { + t.Fatalf("expected id c1, got %v", body["id"]) + } +} + func TestJSONResponseStep_NoWriter(t *testing.T) { factory := NewJSONResponseStepFactory() step, err := factory("no-writer", map[string]any{ diff --git a/module/pipeline_step_request_parse.go b/module/pipeline_step_request_parse.go index 5553e7b39..fdc0ad57b 100644 --- a/module/pipeline_step_request_parse.go +++ b/module/pipeline_step_request_parse.go @@ -47,6 +47,9 @@ func NewRequestParseStepFactory() StepFactory { } parseBody, _ := config["parse_body"].(bool) + if format, _ := config["format"].(string); strings.EqualFold(format, "json") || strings.EqualFold(format, "form") { + parseBody = true + } var parseHeaders []string if ph, ok := config["parse_headers"]; ok { diff --git a/module/pipeline_step_request_parse_test.go b/module/pipeline_step_request_parse_test.go index aa1cdd420..e30785970 100644 --- a/module/pipeline_step_request_parse_test.go +++ b/module/pipeline_step_request_parse_test.go @@ -98,6 +98,37 @@ func TestRequestParseStep_ParseBody(t *testing.T) { } } +func TestRequestParseStep_FormatJSONAlias(t *testing.T) { + factory := NewRequestParseStepFactory() + step, err := factory("parse-body", map[string]any{ + "format": "json", + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + body := bytes.NewBufferString(`{"name":"Acme Corp","slug":"acme"}`) + req, _ := http.NewRequest("POST", "/api/v1/companies", body) + req.Header.Set("Content-Type", "application/json") + + pc := NewPipelineContext(nil, map[string]any{ + "_http_request": req, + }) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute error: %v", err) + } + + bodyData, ok := result.Output["body"].(map[string]any) + if !ok { + t.Fatal("expected body in output") + } + if bodyData["name"] != "Acme Corp" { + t.Errorf("expected name='Acme Corp', got %v", bodyData["name"]) + } +} + func TestRequestParseStep_MultiplePathParams(t *testing.T) { factory := NewRequestParseStepFactory() step, err := factory("parse-multi", map[string]any{ diff --git a/plugins/pipelinesteps/plugin.go b/plugins/pipelinesteps/plugin.go index 3546d54f2..4c5c2bc4e 100644 --- a/plugins/pipelinesteps/plugin.go +++ b/plugins/pipelinesteps/plugin.go @@ -1,6 +1,7 @@ // Package pipelinesteps provides a plugin that registers generic pipeline step // types: validate, transform, conditional, set, log, delegate, jq, publish, // http_call, http_proxy, request_parse, db_query, db_exec, db_query_cached, json_response, +// response, // raw_response, json_parse, static_file, validate_path_param, validate_pagination, // validate_request_body, foreach, while, webhook_verify, base64_decode, ui_scaffold, // ui_scaffold_analyze, dlq_send, dlq_replay, retry_with_backoff, circuit_breaker (wrapping), @@ -70,6 +71,7 @@ func New() *Plugin { "step.db_create_partition", "step.db_sync_partitions", "step.json_response", + "step.response", "step.raw_response", "step.pipeline_output", "step.json_parse", @@ -157,6 +159,7 @@ func (p *Plugin) StepFactories() map[string]plugin.StepFactory { "step.db_create_partition": wrapStepFactory(module.NewDBCreatePartitionStepFactory()), "step.db_sync_partitions": wrapStepFactory(module.NewDBSyncPartitionsStepFactory()), "step.json_response": wrapStepFactory(module.NewJSONResponseStepFactory()), + "step.response": wrapStepFactory(module.NewJSONResponseStepFactory()), "step.raw_response": wrapStepFactory(module.NewRawResponseStepFactory()), "step.pipeline_output": wrapStepFactory(module.NewPipelineOutputStepFactory()), "step.json_parse": wrapStepFactory(module.NewJSONParseStepFactory()), diff --git a/plugins/pipelinesteps/plugin_test.go b/plugins/pipelinesteps/plugin_test.go index 73538b398..4f144d1d4 100644 --- a/plugins/pipelinesteps/plugin_test.go +++ b/plugins/pipelinesteps/plugin_test.go @@ -49,6 +49,7 @@ func TestStepFactories(t *testing.T) { "step.db_create_partition", "step.db_sync_partitions", "step.json_response", + "step.response", "step.raw_response", "step.pipeline_output", "step.json_parse", @@ -97,6 +98,39 @@ func TestStepFactories(t *testing.T) { } } +func TestStepResponseAlias(t *testing.T) { + p := New() + factories := p.StepFactories() + alias, ok := factories["step.response"] + if !ok { + t.Fatal("missing step.response factory") + } + canonical, ok := factories["step.json_response"] + if !ok { + t.Fatal("missing step.json_response factory") + } + + aliasStep, err := alias("respond", map[string]any{"body": map[string]any{"ok": true}}, nil) + if err != nil { + t.Fatalf("alias factory error: %v", err) + } + canonicalStep, err := canonical("respond", map[string]any{"body": map[string]any{"ok": true}}, nil) + if err != nil { + t.Fatalf("canonical factory error: %v", err) + } + aliasNamed, ok := aliasStep.(interface{ Name() string }) + if !ok { + t.Fatalf("alias step does not expose Name(): %T", aliasStep) + } + canonicalNamed, ok := canonicalStep.(interface{ Name() string }) + if !ok { + t.Fatalf("canonical step does not expose Name(): %T", canonicalStep) + } + if aliasNamed.Name() != canonicalNamed.Name() { + t.Fatalf("alias step name = %q, want %q", aliasNamed.Name(), canonicalNamed.Name()) + } +} + func TestPluginLoads(t *testing.T) { p := New() loader := plugin.NewPluginLoader(capability.NewRegistry(), schema.NewModuleSchemaRegistry()) diff --git a/schema/module_schema.go b/schema/module_schema.go index 5eafffc32..8e7a8c607 100644 --- a/schema/module_schema.go +++ b/schema/module_schema.go @@ -928,9 +928,12 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { Inputs: []ServiceIODef{{Name: "context", Type: "PipelineContext", Description: "Pipeline context with field to evaluate for routing"}}, Outputs: []ServiceIODef{{Name: "result", Type: "StepResult", Description: "Routing decision with target step name"}}, ConfigFields: []ConfigFieldDef{ - {Key: "field", Label: "Field", Type: FieldTypeString, Required: true, Description: "Field path to evaluate for routing", Placeholder: "event_type"}, - {Key: "routes", Label: "Routes", Type: FieldTypeMap, MapValueType: "string", Required: true, Description: "Map of field values to target step names"}, + {Key: "field", Label: "Field", Type: FieldTypeString, Description: "Field path to evaluate for switch-style routing", Placeholder: "event_type"}, + {Key: "routes", Label: "Routes", Type: FieldTypeMap, MapValueType: "string", Description: "Map of field values to target step names"}, {Key: "default", Label: "Default Step", Type: FieldTypeString, Description: "Step name to route to when no match is found"}, + {Key: "if", Label: "If", Type: FieldTypeString, Description: "Boolean condition for if/then/else routing; supports ${ } expressions and Go template truthy output", Placeholder: `${ status == "active" }`}, + {Key: "then", Label: "Then Step", Type: FieldTypeString, Description: "Step name to route to when if evaluates truthy"}, + {Key: "else", Label: "Else Step", Type: FieldTypeString, Description: "Step name to route to when if evaluates false"}, }, }) @@ -1027,7 +1030,9 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { ConfigFields: []ConfigFieldDef{ {Key: "path_params", Label: "Path Parameters", Type: FieldTypeArray, ArrayItemType: "string", Description: "Parameter names to extract from URL path (e.g., id, companyId)"}, {Key: "query_params", Label: "Query Parameters", Type: FieldTypeArray, ArrayItemType: "string", Description: "Query string parameter names to extract"}, - {Key: "parse_body", Label: "Parse Body", Type: FieldTypeBool, Description: "Whether to parse the JSON request body"}, + {Key: "parse_body", Label: "Parse Body", Type: FieldTypeBool, Description: "Whether to parse the request body"}, + {Key: "format", Label: "Format Alias", Type: FieldTypeSelect, Options: []string{"json", "form"}, Description: "Alias that enables body parsing for JSON or form request bodies"}, + {Key: "parse_headers", Label: "Parse Headers", Type: FieldTypeArray, ArrayItemType: "string", Description: "Header names to extract"}, }, }) @@ -1133,6 +1138,22 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { }, }) + r.Register(&ModuleSchema{ + Type: "step.response", + Label: "Response", + Category: "pipeline", + Description: "Alias for step.json_response; writes an HTTP JSON response with custom status code and stops the pipeline", + Inputs: []ServiceIODef{{Name: "context", Type: "PipelineContext", Description: "Pipeline context with _http_response_writer metadata"}}, + Outputs: []ServiceIODef{{Name: "result", Type: "StepResult", Description: "Response status (always sets Stop: true)"}}, + ConfigFields: []ConfigFieldDef{ + {Key: "status", Label: "Status Code", Type: FieldTypeNumber, DefaultValue: "200", Description: "HTTP status code for the response"}, + {Key: "status_from", Label: "Status From", Type: FieldTypeString, Description: "Dotted path to resolve HTTP status code dynamically (e.g., steps.call_upstream.status_code). Takes precedence over 'status' when resolved to a valid HTTP status code (100-599).", Placeholder: "steps.call_upstream.status_code"}, + {Key: "headers", Label: "Headers", Type: FieldTypeMap, MapValueType: "string", Description: "Additional response headers"}, + {Key: "body", Label: "Body", Type: FieldTypeMap, Description: "Response body as JSON (supports template expressions)"}, + {Key: "body_from", Label: "Body From", Type: FieldTypeString, Description: "Dotted path to resolve body from step outputs (e.g., steps.get-company.row)", Placeholder: "steps.get-company.row"}, + }, + }) + r.Register(&ModuleSchema{ Type: "step.jq", Label: "JQ Transform", From 3b84e5dcd34d2bdf060b4b7f81fa55a185314985 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 00:50:20 -0400 Subject: [PATCH 09/10] docs: remove config quirks guide --- DOCUMENTATION.md | 1 + cmd/wfctl/modernize_test.go | 1 + docs/WFCTL.md | 6 + docs/config-field-quirks.md | 226 ------------------ docs/dsl-reference.md | 11 + docs/tutorials/building-apps-with-workflow.md | 12 + schema/module_schema_test.go | 2 +- schema/schema.go | 1 + schema/step_schema_builtins.go | 16 ++ schema/testdata/editor-schemas.golden.json | 186 ++++++++++++-- 10 files changed, 219 insertions(+), 243 deletions(-) delete mode 100644 docs/config-field-quirks.md diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 1bd85046b..7955f5b6c 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -156,6 +156,7 @@ flowchart TD | `step.db_create_partition` | Creates a time-based table partition | pipelinesteps | | `step.db_sync_partitions` | Ensures future partitions exist for a partitioned table | pipelinesteps | | `step.json_response` | Writes HTTP JSON response with custom status code and headers. Supports `status_from` to dynamically resolve the HTTP status code from the pipeline context at runtime | pipelinesteps | +| `step.response` | Alias for `step.json_response` for concise pipeline-authored HTTP JSON responses | pipelinesteps | | `step.raw_response` | Writes a raw HTTP response with arbitrary content type | pipelinesteps | | `step.pipeline_output` | Marks structured data as the pipeline's return value for extraction by `engine.ExecutePipeline()` | pipelinesteps | | `step.json_parse` | Parses a JSON string (or `[]byte`) in the pipeline context into a structured object | pipelinesteps | diff --git a/cmd/wfctl/modernize_test.go b/cmd/wfctl/modernize_test.go index 86583e39b..724753037 100644 --- a/cmd/wfctl/modernize_test.go +++ b/cmd/wfctl/modernize_test.go @@ -645,6 +645,7 @@ func TestModernizeAllRulesRegistered(t *testing.T) { "absolute-dbpath", "empty-routes", "camelcase-config", + "db-config-aliases", "request-parse-config", "legacy-do-types", "legacy-aws-types", diff --git a/docs/WFCTL.md b/docs/WFCTL.md index 5b2cf9ce5..212c002bb 100644 --- a/docs/WFCTL.md +++ b/docs/WFCTL.md @@ -2812,12 +2812,18 @@ wfctl modernize [options] |----|----------|---------|-------------| | `hyphen-steps` | error | yes | Rename hyphenated step names to underscores | | `conditional-field` | error | yes | Convert template syntax in conditional field to dot-path | +| `db-config-aliases` | warning | yes | Rewrite DB step aliases (`module`, `args`, `one`, `many`) to canonical config | | `db-query-mode` | warning | yes | Add mode:single when downstream uses .row/.found | | `db-query-index` | error | yes | Convert .steps.X.row.Y to index syntax | | `database-to-sqlite` | warning | yes | Convert database.workflow to storage.sqlite | | `absolute-dbpath` | warning | no | Warn on absolute dbPath values | | `empty-routes` | error | no | Detect empty routes in step.conditional | | `camelcase-config` | warning | no | Detect snake_case config keys | +| `request-parse-config` | warning | no | Detect request parsing config that should be reviewed | + +Modernize rewrites accepted compatibility aliases back to canonical config where +a deterministic fix exists. Runtime compatibility remains additive; canonical +docs and generated schemas prefer the stable spelling. **Plugin-provided rules** are loaded when `--plugin-dir` is specified. Each installed plugin can declare its own migration rules in its `plugin.json` manifest under the `modernizeRules` key. Rules from all plugins in the directory are merged with the built-in rules. diff --git a/docs/config-field-quirks.md b/docs/config-field-quirks.md deleted file mode 100644 index fdf842a8b..000000000 --- a/docs/config-field-quirks.md +++ /dev/null @@ -1,226 +0,0 @@ -# Workflow Config Field Quirks & Inconsistencies - -**Date:** 2026-04-13 -**Context:** Discovered during self-improving agentic workflow scenario execution. These inconsistencies cause LLM-generated configs to fail at startup, requiring iterative validation to correct. - -## Critical: Field Names That Don't Match Intuition - -### 1. `http.server` uses `address` not `port` - -**Wrong:** -```yaml -- name: server - type: http.server - config: - port: 8080 # FAILS: "required config field 'address' is missing" -``` - -**Correct:** -```yaml -- name: server - type: http.server - config: - address: ":8080" # Note: string with colon prefix, not integer -``` - -**Impact:** Every LLM and most humans guess `port: 8080`. The field name `address` is non-obvious, and the value format (`:8080` string vs `8080` integer) is also unexpected. - -### 2. `step.db_exec` / `step.db_query` use `database` not `module` - -**Wrong:** -```yaml -- name: insert - type: step.db_exec - config: - module: db # FAILS: "'database' is required" -``` - -**Correct:** -```yaml -- name: insert - type: step.db_exec - config: - database: db # References the module by name -``` - -**Impact:** Since the value references a module name, `module:` is the intuitive field name. `database:` is the storage-specific name. - -### 3. `step.db_exec` / `step.db_query` use `params` not `args` - -**Wrong:** -```yaml -config: - query: "INSERT INTO tasks (title) VALUES (?)" - args: # FAILS: "missing argument with index 1" - - "{{ .body.title }}" -``` - -**Correct:** -```yaml -config: - query: "INSERT INTO tasks (title) VALUES (?)" - params: # Named "params" not "args" - - "{{ .body.title }}" -``` - -**Impact:** `args` is the more common name in most frameworks and languages. `params` works but isn't obvious. - -### 4. `step.db_query` mode values: `list`/`single` not `many`/`one` - -**Wrong:** -```yaml -config: - mode: many # FAILS: "mode must be 'list' or 'single'" - mode: one # Also fails -``` - -**Correct:** -```yaml -config: - mode: list # For multiple rows - mode: single # For one row -``` - -**Impact:** `many`/`one` or `multiple`/`single` are intuitive pairs. `list`/`single` is an asymmetric naming pattern. - -### 5. `step.request_parse` uses `parse_body: true` not `format: json` - -**Wrong:** -```yaml -- name: parse - type: step.request_parse - config: - format: json # Silently ignored — body not parsed -``` - -**Correct:** -```yaml -- name: parse - type: step.request_parse - config: - parse_body: true # Boolean flag, format auto-detected from Content-Type -``` - -**Impact:** Most HTTP frameworks use `format` or `content_type` to specify body parsing. The boolean `parse_body` flag is unusual and doesn't indicate what format is expected. - -### 6. Response step is `step.json_response` not `step.response` - -**Wrong:** -```yaml -- name: respond - type: step.response # FAILS: "unknown step type: step.response" -``` - -**Correct:** -```yaml -- name: respond - type: step.json_response # Must specify json_ prefix -``` - -**Impact:** `step.response` is the obvious name. Having to know it's `step.json_response` requires checking the step registry. There's no generic `step.response` — you must choose `step.json_response`, `step.html_response`, etc. - -## Important: Structural Patterns That Surprise - -### 7. Pipeline triggers are inline, not workflow-level routes - -**Wrong (routes pointing to pipelines):** -```yaml -workflows: - http: - routes: - - path: /tasks - method: GET - pipeline: list_tasks # NOT how it works -pipelines: - list_tasks: - steps: [...] -``` - -**Correct (inline trigger in pipeline):** -```yaml -workflows: - http: - routes: [] # Empty — routes come from triggers -pipelines: - list_tasks: - trigger: - type: http - config: - path: /tasks - method: GET - steps: [...] -``` - -**Impact:** This is the most confusing structural pattern. Most frameworks define routes in one place and point to handlers. Workflow inverts this — each pipeline declares its own trigger. The `routes: []` in the workflow section is required but counterintuitive since it's always empty when using inline triggers. - -### 8. `step.json_response` body can be object or template string - -```yaml -# Object form — produces clean JSON: -body: - status: healthy - version: "1.0" - -# Template form — produces JSON string (may double-serialize): -body: "{{ .steps.query.rows | json }}" -``` - -**Impact:** The template form produces a JSON string value, not raw JSON. List endpoints return `"[{...}]"` (a JSON string containing JSON) instead of `[{...}]` (a JSON array). Consumers must parse twice. - -### 9. `step.conditional` uses `field` + `routes` map + `default`, not `condition`/`then`/`else` - -**Wrong:** -```yaml -- name: check - type: step.conditional - config: - condition: "{{ .status == 'active' }}" - then: proceed - else: reject -``` - -**Correct:** -```yaml -- name: check - type: step.conditional - config: - field: "{{ .status }}" - routes: - active: proceed - inactive: reject - default: reject # REQUIRED — omitting causes error -``` - -**Impact:** `condition`/`then`/`else` is the universal conditional pattern. The `field` + `routes` map pattern is more like a switch statement, which is unusual for a step named "conditional". - -### 10. Template data access varies by source - -```yaml -# Path params are flat: -"{{ .id }}" # NOT {{ .path.id }} - -# Query params are flat: -"{{ .q }}" # NOT {{ .query.q }} - -# Request body requires parse step: -"{{ .body.title }}" # After step.request_parse - -# Step outputs use index for hyphens: -"{{ index .steps \"my-step\" \"key\" }}" # NOT {{ .steps.my-step.key }} - -# But dot-access works for underscore names: -"{{ .steps.my_step.key }}" # Works fine -``` - -**Impact:** Inconsistent data access patterns make template authoring error-prone. Flat path/query params are convenient but surprising. The hyphenated step name issue is documented but easy to forget. - -## Recommendations - -1. **Add aliases** — Accept `port` as alias for `address`, `module` as alias for `database`, `args` as alias for `params`, `many`/`one` as aliases for `list`/`single` -2. **Register `step.response`** as alias for `step.json_response` (most common case) -3. **Fix double-serialization** — Template `body` expressions should produce raw values, not JSON strings -4. **Add `step.request_parse` format param** — Accept `format: json` as equivalent to `parse_body: true` -5. **Document route pattern explicitly** — The inline trigger pattern is the biggest structural surprise; needs prominent documentation -6. **Add `wfctl modernize` rules** — Detect and auto-fix common misnamings (port→address, module→database, args→params) -7. **Enhance LSP completions** — When typing a wrong field name, suggest the correct one -8. **Consider `step.conditional` `if`/`then`/`else` mode** — Add alongside existing `field`/`routes` for simple boolean conditions diff --git a/docs/dsl-reference.md b/docs/dsl-reference.md index b72a4cb88..f87f8c0b8 100644 --- a/docs/dsl-reference.md +++ b/docs/dsl-reference.md @@ -104,6 +104,10 @@ modules: - `dependsOn` forms a DAG — circular dependencies are rejected at startup - Module types are registered by plugins declared in `requires.plugins` +### Canonical Field Notes +- `http.server.config.address` is the canonical listen address. `port: 8080` is accepted as an authoring alias and normalizes to `":8080"` when `address` is omitted. +- Use `wfctl modernize --apply` to rewrite accepted aliases back to canonical fields before committing long-lived config. + --- @@ -410,9 +414,16 @@ The original `{{ }}` syntax is still fully supported: | `step.conditional` | Branch execution based on a field value | | `step.http_call` | Make an outbound HTTP request | | `step.json_response` | Send a JSON HTTP response and stop pipeline | +| `step.response` | Alias for `step.json_response` | | `step.db_query` | Execute a SQL query against a database module | | `step.publish` | Publish a message to a messaging topic | +### Pipeline Authoring Notes +- `step.db_query`, `step.db_exec`, and `step.db_query_cached` use `database` and `params` as canonical keys. The aliases `module` and `args` are accepted, and `mode: one`/`many` normalize to `single`/`list`. +- `step.request_parse` uses `parse_body: true` as the canonical body parsing flag. `format: json` and `format: form` are accepted aliases that enable body parsing. +- `step.conditional` supports switch-style `field`/`routes`/`default` and boolean `if`/`then`/`else` routing. `${ status == "active" }` is the preferred boolean expression syntax. +- `step.json_response` and its `step.response` alias encode template results that resolve to JSON arrays or objects as raw JSON values rather than strings. + ### Example ```yaml pipelines: diff --git a/docs/tutorials/building-apps-with-workflow.md b/docs/tutorials/building-apps-with-workflow.md index 9ba583bbd..c1134a71b 100644 --- a/docs/tutorials/building-apps-with-workflow.md +++ b/docs/tutorials/building-apps-with-workflow.md @@ -253,6 +253,7 @@ Available step types: | `step.db_query` | Execute a SELECT query against a database module | | `step.db_exec` | Execute INSERT/UPDATE/DELETE against a database module | | `step.json_response` | Return a JSON HTTP response | +| `step.response` | Alias for `step.json_response` | | `step.http_call` | Make an outbound HTTP request | | `step.publish` | Publish a message to a broker or EventBus topic | | `step.state_transition` | Trigger a state machine transition | @@ -263,6 +264,17 @@ Available step types: | `step.script` | Execute a Yaegi dynamic script | | `step.notify` | Send notifications (Slack, email, webhook) | +Canonical examples in this guide use `address`, `database`, `params`, +`mode: list`/`single`, `parse_body`, and `step.json_response`. Workflow also +accepts common aliases such as `port`, `module`, `args`, `mode: many`/`one`, +`format: json`, and `step.response`. Run `wfctl modernize --apply ` to +rewrite aliases to canonical fields before committing shared configuration. + +Pipeline HTTP routes are usually declared inline on each pipeline with +`trigger.type: http`. This keeps the route next to the steps that handle it. +`workflows.http.routes` remains available for module-handler routes; the two +models are additive rather than competing. + --- ## 3. Your First Application (Step-by-Step) diff --git a/schema/module_schema_test.go b/schema/module_schema_test.go index 3979d87e1..2d5588034 100644 --- a/schema/module_schema_test.go +++ b/schema/module_schema_test.go @@ -56,7 +56,7 @@ func TestModuleSchemaRegistry_ConfigFieldsMatchEngine(t *testing.T) { moduleType string wantFields []string }{ - {"http.server", []string{"address"}}, + {"http.server", []string{"address", "port"}}, {"http.handler", []string{"contentType"}}, {"http.middleware.ratelimit", []string{"requestsPerMinute", "burstSize"}}, {"http.middleware.cors", []string{"allowedOrigins", "allowedMethods"}}, diff --git a/schema/schema.go b/schema/schema.go index 50b52622b..3c8f97421 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -366,6 +366,7 @@ var coreModuleTypes = []string{ "step.region_weight", "step.request_parse", "step.resilient_circuit_breaker", + "step.response", "step.retry_with_backoff", "step.s3_upload", "step.sandbox_exec", diff --git a/schema/step_schema_builtins.go b/schema/step_schema_builtins.go index 2bf2bfc5c..c47f3b782 100644 --- a/schema/step_schema_builtins.go +++ b/schema/step_schema_builtins.go @@ -108,6 +108,22 @@ func (r *StepSchemaRegistry) registerBuiltins() { }, }) + r.Register(&StepSchema{ + Type: "step.response", + Plugin: "pipelinesteps", + Description: "Alias for step.json_response; sends a JSON HTTP response and terminates pipeline execution.", + ConfigFields: []ConfigFieldDef{ + {Key: "status", Type: FieldTypeNumber, Description: "HTTP status code (default 200)", DefaultValue: "200"}, + {Key: "status_from", Type: FieldTypeString, Description: "Dotted path to resolve HTTP status code dynamically (e.g. 'steps.call_upstream.status_code'). Takes precedence over 'status' when resolved to a valid HTTP status code (100-599)."}, + {Key: "body", Type: FieldTypeMap, Description: "Response body (static JSON object or template expression)"}, + {Key: "body_from", Type: FieldTypeString, Description: "Template expression to build body from step outputs (e.g. 'steps.query.rows')"}, + {Key: "headers", Type: FieldTypeMap, Description: "Additional response headers"}, + }, + Outputs: []StepOutputDef{ + {Key: "sent", Type: "boolean", Description: "Whether the response was sent successfully"}, + }, + }) + r.Register(&StepSchema{ Type: "step.request_parse", Plugin: "pipelinesteps", diff --git a/schema/testdata/editor-schemas.golden.json b/schema/testdata/editor-schemas.golden.json index 3f1fd9b85..cdcd85617 100644 --- a/schema/testdata/editor-schemas.golden.json +++ b/schema/testdata/editor-schemas.golden.json @@ -1804,10 +1804,16 @@ "key": "address", "label": "Listen Address", "type": "string", - "description": "Host:port to listen on (e.g. :8080, 0.0.0.0:80)", - "required": true, + "description": "Canonical host:port to listen on (e.g. :8080, 0.0.0.0:80)", "defaultValue": ":8080", "placeholder": ":8080" + }, + { + "key": "port", + "label": "Port Alias", + "type": "number", + "description": "Alias for address; normalized to :\u003cport\u003e when address is omitted", + "placeholder": "8080" } ], "defaultConfig": { @@ -4433,8 +4439,7 @@ "key": "field", "label": "Field", "type": "string", - "description": "Field path to evaluate for routing", - "required": true, + "description": "Field path to evaluate for switch-style routing", "placeholder": "event_type" }, { @@ -4442,7 +4447,6 @@ "label": "Routes", "type": "map", "description": "Map of field values to target step names", - "required": true, "mapValueType": "string" }, { @@ -4450,6 +4454,25 @@ "label": "Default Step", "type": "string", "description": "Step name to route to when no match is found" + }, + { + "key": "if", + "label": "If", + "type": "string", + "description": "Boolean condition for if/then/else routing; supports ${ } expressions and Go template truthy output", + "placeholder": "${ status == \"active\" }" + }, + { + "key": "then", + "label": "Then Step", + "type": "string", + "description": "Step name to route to when if evaluates truthy" + }, + { + "key": "else", + "label": "Else Step", + "type": "string", + "description": "Step name to route to when if evaluates false" } ] }, @@ -4603,8 +4626,15 @@ "key": "database", "label": "Database", "type": "string", - "description": "Name of the database service (must implement DBProvider)", - "required": true, + "description": "Canonical name of the database service (must implement DBProvider)", + "placeholder": "admin-db", + "inheritFrom": "dependency.name" + }, + { + "key": "module", + "label": "Database Alias", + "type": "string", + "description": "Alias for database; wfctl modernize rewrites this to database", "placeholder": "admin-db", "inheritFrom": "dependency.name" }, @@ -4623,6 +4653,25 @@ "description": "Template-resolved parameter values for ? placeholders", "arrayItemType": "string" }, + { + "key": "args", + "label": "Parameters Alias", + "type": "array", + "description": "Alias for params; wfctl modernize rewrites this to params", + "arrayItemType": "string" + }, + { + "key": "mode", + "label": "Mode", + "type": "select", + "description": "Result mode for returning statements: list/many returns rows/count, single/one returns row/found", + "options": [ + "list", + "single", + "many", + "one" + ] + }, { "key": "tenantKey", "label": "Tenant Key", @@ -4663,8 +4712,15 @@ "key": "database", "label": "Database", "type": "string", - "description": "Name of the database service (must implement DBProvider)", - "required": true, + "description": "Canonical name of the database service (must implement DBProvider)", + "placeholder": "admin-db", + "inheritFrom": "dependency.name" + }, + { + "key": "module", + "label": "Database Alias", + "type": "string", + "description": "Alias for database; wfctl modernize rewrites this to database", "placeholder": "admin-db", "inheritFrom": "dependency.name" }, @@ -4683,15 +4739,24 @@ "description": "Template-resolved parameter values for ? placeholders in query", "arrayItemType": "string" }, + { + "key": "args", + "label": "Parameters Alias", + "type": "array", + "description": "Alias for params; wfctl modernize rewrites this to params", + "arrayItemType": "string" + }, { "key": "mode", "label": "Mode", "type": "select", - "description": "Result mode: 'list' returns rows/count, 'single' returns row/found", + "description": "Result mode: list/many returns rows/count, single/one returns row/found", "defaultValue": "list", "options": [ "list", - "single" + "single", + "many", + "one" ] }, { @@ -4734,8 +4799,15 @@ "key": "database", "label": "Database", "type": "string", - "description": "Name of the database service (must implement DBProvider)", - "required": true, + "description": "Canonical name of the database service (must implement DBProvider)", + "placeholder": "db", + "inheritFrom": "dependency.name" + }, + { + "key": "module", + "label": "Database Alias", + "type": "string", + "description": "Alias for database; wfctl modernize rewrites this to database", "placeholder": "db", "inheritFrom": "dependency.name" }, @@ -4754,15 +4826,24 @@ "description": "Template-resolved parameter values for query placeholders", "arrayItemType": "string" }, + { + "key": "args", + "label": "Parameters Alias", + "type": "array", + "description": "Alias for params; wfctl modernize rewrites this to params", + "arrayItemType": "string" + }, { "key": "mode", "label": "Mode", "type": "select", - "description": "Result mode: 'single' returns row/found, 'list' returns rows/count", + "description": "Result mode: single/one returns row/found, list/many returns rows/count", "defaultValue": "single", "options": [ "single", - "list" + "list", + "one", + "many" ] }, { @@ -6924,7 +7005,24 @@ "key": "parse_body", "label": "Parse Body", "type": "boolean", - "description": "Whether to parse the JSON request body" + "description": "Whether to parse the request body" + }, + { + "key": "format", + "label": "Format Alias", + "type": "select", + "description": "Alias that enables body parsing for JSON or form request bodies", + "options": [ + "json", + "form" + ] + }, + { + "key": "parse_headers", + "label": "Parse Headers", + "type": "array", + "description": "Header names to extract", + "arrayItemType": "string" } ] }, @@ -6963,6 +7061,62 @@ } ] }, + "step.response": { + "type": "step.response", + "label": "Response", + "category": "pipeline", + "description": "Alias for step.json_response; writes an HTTP JSON response with custom status code and stops the pipeline", + "inputs": [ + { + "name": "context", + "type": "PipelineContext", + "description": "Pipeline context with _http_response_writer metadata" + } + ], + "outputs": [ + { + "name": "result", + "type": "StepResult", + "description": "Response status (always sets Stop: true)" + } + ], + "configFields": [ + { + "key": "status", + "label": "Status Code", + "type": "number", + "description": "HTTP status code for the response", + "defaultValue": "200" + }, + { + "key": "status_from", + "label": "Status From", + "type": "string", + "description": "Dotted path to resolve HTTP status code dynamically (e.g., steps.call_upstream.status_code). Takes precedence over 'status' when resolved to a valid HTTP status code (100-599).", + "placeholder": "steps.call_upstream.status_code" + }, + { + "key": "headers", + "label": "Headers", + "type": "map", + "description": "Additional response headers", + "mapValueType": "string" + }, + { + "key": "body", + "label": "Body", + "type": "map", + "description": "Response body as JSON (supports template expressions)" + }, + { + "key": "body_from", + "label": "Body From", + "type": "string", + "description": "Dotted path to resolve body from step outputs (e.g., steps.get-company.row)", + "placeholder": "steps.get-company.row" + } + ] + }, "step.retry_with_backoff": { "type": "step.retry_with_backoff", "label": "Retry With Backoff", From ce222aef9fffc90d92cda01f36ad65bea454022c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 7 Jun 2026 01:06:11 -0400 Subject: [PATCH 10/10] fix: address config alias review --- cmd/wfctl/type_registry.go | 4 +-- cmd/wfctl/type_registry_test.go | 48 +++++++++++++++++++++++++++++++ schema/schema_test.go | 30 ++++++++++++++++++++ schema/validate.go | 50 +++++++++++++++++++++++++++++++-- 4 files changed, 128 insertions(+), 4 deletions(-) diff --git a/cmd/wfctl/type_registry.go b/cmd/wfctl/type_registry.go index d50cc7c09..b1c27661c 100644 --- a/cmd/wfctl/type_registry.go +++ b/cmd/wfctl/type_registry.go @@ -772,7 +772,7 @@ func KnownStepTypes() map[string]StepTypeInfo { "step.request_parse": { Type: "step.request_parse", Plugin: "pipelinesteps", - ConfigKeys: []string{"body", "query", "headers", "path_params", "query_params", "parse_body", "parse_headers", "format"}, + ConfigKeys: []string{"path_params", "query_params", "parse_body", "parse_headers", "format"}, }, "step.db_query": { Type: "step.db_query", @@ -802,7 +802,7 @@ func KnownStepTypes() map[string]StepTypeInfo { "step.json_response": { Type: "step.json_response", Plugin: "pipelinesteps", - ConfigKeys: []string{"status", "body", "headers"}, + ConfigKeys: []string{"status", "status_from", "body", "body_from", "headers"}, }, "step.response": { Type: "step.response", diff --git a/cmd/wfctl/type_registry_test.go b/cmd/wfctl/type_registry_test.go index d3486a2f1..9a6fa26b8 100644 --- a/cmd/wfctl/type_registry_test.go +++ b/cmd/wfctl/type_registry_test.go @@ -204,6 +204,54 @@ func TestStepTypeCount(t *testing.T) { } } +func TestKnownStepTypesRequestParseConfigKeys(t *testing.T) { + info, ok := KnownStepTypes()["step.request_parse"] + if !ok { + t.Fatal("step.request_parse not found") + } + want := []string{"path_params", "query_params", "parse_body", "parse_headers", "format"} + if !sameStringSet(info.ConfigKeys, want) { + t.Fatalf("step.request_parse ConfigKeys = %v, want %v", info.ConfigKeys, want) + } +} + +func TestKnownStepTypesResponseAliasesMatchConfigKeys(t *testing.T) { + types := KnownStepTypes() + jsonInfo, ok := types["step.json_response"] + if !ok { + t.Fatal("step.json_response not found") + } + aliasInfo, ok := types["step.response"] + if !ok { + t.Fatal("step.response not found") + } + if !sameStringSet(jsonInfo.ConfigKeys, aliasInfo.ConfigKeys) { + t.Fatalf("step.json_response ConfigKeys = %v, step.response ConfigKeys = %v", jsonInfo.ConfigKeys, aliasInfo.ConfigKeys) + } +} + +func sameStringSet(got, want []string) bool { + if len(got) != len(want) { + return false + } + seen := make(map[string]int, len(got)) + for _, v := range got { + seen[v]++ + } + for _, v := range want { + if seen[v] == 0 { + return false + } + seen[v]-- + } + for _, count := range seen { + if count != 0 { + return false + } + } + return true +} + // TestKnownStepTypesCoverAllPlugins ensures KnownStepTypes() is in sync with all step // types registered by the built-in plugins. Any step type registered by a DefaultPlugin // that is not listed in KnownStepTypes() will cause this test to fail, preventing silent diff --git a/schema/schema_test.go b/schema/schema_test.go index c76e18528..ef9d340b8 100644 --- a/schema/schema_test.go +++ b/schema/schema_test.go @@ -377,6 +377,36 @@ func TestValidateConfig_HTTPServer_ValidPortAlias(t *testing.T) { } } +func TestValidateConfig_HTTPServer_InvalidPortAlias(t *testing.T) { + tests := []struct { + name string + port any + }{ + {name: "non-numeric", port: "abc"}, + {name: "nil", port: nil}, + {name: "too-low", port: 0}, + {name: "too-high", port: 70000}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &config.WorkflowConfig{ + Modules: []config.ModuleConfig{ + {Name: "srv", Type: "http.server", Config: map[string]any{"port": tt.port}}, + }, + Triggers: map[string]any{ + "http": map[string]any{"port": 9090}, + }, + } + err := ValidateConfig(cfg) + if err == nil { + t.Fatal("expected invalid port error") + } + assertContains(t, err.Error(), "modules[0].config.port") + assertContains(t, err.Error(), "valid TCP port") + }) + } +} + func TestValidateConfig_StaticFileserver_MissingRoot(t *testing.T) { cfg := &config.WorkflowConfig{ Modules: []config.ModuleConfig{ diff --git a/schema/validate.go b/schema/validate.go index d7c3ea38f..2297d1f13 100644 --- a/schema/validate.go +++ b/schema/validate.go @@ -2,6 +2,7 @@ package schema import ( "fmt" + "strconv" "strings" "unicode" @@ -327,12 +328,22 @@ func validateModuleConfig(mod config.ModuleConfig, prefix string, errs *Validati break } address, _ := mod.Config["address"].(string) - _, hasPort := mod.Config["port"] - if strings.TrimSpace(address) == "" && !hasPort { + if strings.TrimSpace(address) != "" { + break + } + port, hasPort := mod.Config["port"] + if !hasPort { *errs = append(*errs, &ValidationError{ Path: prefix + ".config", Message: `http.server requires either "address" or "port"`, }) + break + } + if !validTCPPortValue(port) { + *errs = append(*errs, &ValidationError{ + Path: prefix + ".config.port", + Message: "port must be a valid TCP port number between 1 and 65535", + }) } case "database.partitioned": if mod.Config == nil { @@ -425,6 +436,41 @@ func validateModuleConfig(mod config.ModuleConfig, prefix string, errs *Validati } } +func validTCPPortValue(v any) bool { + switch p := v.(type) { + case int: + return p >= 1 && p <= 65535 + case int8: + return p >= 1 + case int16: + return p >= 1 + case int32: + return p >= 1 && p <= 65535 + case int64: + return p >= 1 && p <= 65535 + case uint: + return p >= 1 && p <= 65535 + case uint8: + return p >= 1 + case uint16: + return p >= 1 + case uint32: + return p >= 1 && p <= 65535 + case uint64: + return p >= 1 && p <= 65535 + case float32: + f := float64(p) + return f >= 1 && f <= 65535 && f == float64(int64(f)) + case float64: + return p >= 1 && p <= 65535 && p == float64(int64(p)) + case string: + n, err := strconv.Atoi(strings.TrimSpace(p)) + return err == nil && n >= 1 && n <= 65535 + default: + return false + } +} + // entryPointModuleTypes are module types that inherently serve as entry points // because they listen for external input (HTTP servers, schedulers, event buses). var entryPointModuleTypes = map[string]bool{