feat: add step with config alias#2021
Merged
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request renames the executor configuration field from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/dagu/references/codingagent.md (1)
68-68:⚠️ Potential issue | 🟡 MinorUpdate the remaining legacy
configwording.This line still says “config key” in the
withdocs; use “withkey” to avoid reintroducing the deprecated name.📝 Proposed wording fix
-- `option_flags` — per-option override from config key to exact flag token +- `option_flags` — per-option override from `with` key to exact flag token🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/dagu/references/codingagent.md` at line 68, The docs line for option_flags still uses the deprecated phrase "config key"; update the wording in the `option_flags` entry of the `with` docs from "config key" to "`with` key" so it reads "`option_flags` — per-option override from `with` key to exact flag token", ensuring the documentation consistently uses the `with` terminology.
🧹 Nitpick comments (2)
internal/service/frontend/api/v1/dags_internal_test.go (1)
189-191: LGTM — test input migrated towith.Using the canonical
with:key here exercises the new naming end-to-end throughUpdateDAGSpec. Consider adding a sibling test that still uses the legacyconfig:alias to lock in backward compatibility, and one that sets both to assert the validation rejection path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/service/frontend/api/v1/dags_internal_test.go` around lines 189 - 191, Add two new tests alongside the existing test that uses the canonical with: key: one that sends the legacy config: alias as input and asserts UpdateDAGSpec still accepts it, and another that sets both with: and config: in the same payload and asserts UpdateDAGSpec returns a validation error; place them in internal/service/frontend/api/v1/dags_internal_test.go near the existing "with:" test so they exercise the same test harness and call the same UpdateDAGSpec helper/endpoint used by the current test. Ensure the legacy test constructs the payload using "config:" instead of "with:" and asserts success, while the conflict test constructs a payload containing both keys and asserts the API responds with the expected validation failure.internal/core/spec/step_types_test.go (1)
93-118: Add handler call-site coverage for thewith/configconflict.The new conflict test covers normal
steps, whilehandler_on.successis also a custom step call site. A negative handler case would lock the same invariant on that path.🧪 Proposed test coverage addition
func TestCustomStepTypes_RejectWithAndLegacyConfig(t *testing.T) { t.Parallel() _, err := LoadYAML(context.Background(), []byte(` @@ require.Error(t, err) assert.Contains(t, err.Error(), `fields "with" and "config" cannot be used together`) } + +func TestCustomStepTypes_HandlerRejectsWithAndLegacyConfig(t *testing.T) { + t.Parallel() + + _, err := LoadYAML(context.Background(), []byte(` +name: custom-step-handler-mixed-config +step_types: + greet: + type: command + input_schema: + type: object + additionalProperties: false + properties: + message: + type: string + template: + command: echo {{ .input.message }} +handler_on: + success: + type: greet + with: + message: hello + config: + message: goodbye +steps: + - command: echo run +`)) + require.Error(t, err) + assert.Contains(t, err.Error(), `fields "with" and "config" cannot be used together`) +}Also applies to: 493-496
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/core/spec/step_types_test.go` around lines 93 - 118, Add a parallel test similar to TestCustomStepTypes_RejectWithAndLegacyConfig that exercises a handler call-site (e.g., under handler_on.success) using both "with" and "config" together and asserts LoadYAML returns an error containing `fields "with" and "config" cannot be used together`; specifically create a new test (e.g., TestCustomStepTypes_RejectWithAndLegacyConfig_Handler) that calls LoadYAML with YAML where a handler_on.success step invokes the custom step "greet" and includes both a with block and a config block, then require.Error on the result and assert the error message contains the same conflict text so the handler path enforces the same invariant (reuse LoadYAML and the same assert.Contains pattern used in TestCustomStepTypes_RejectWithAndLegacyConfig).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/dagu/references/codingagent.md`:
- Around line 108-110: The wording is ambiguous about where step-level fallback
belongs; clarify that the step-level "fallback" is nested under the step's
"with" block rather than being a top-level step field: update the lines
describing harness config so they read something like "DAG-level primary harness
config is the base; Step-level `with` overlays it; Step-level `with.fallback`
replaces DAG-level `fallback`", and ensure references to `fallback` explicitly
show it as `with.fallback` (or describe "fallback under `with`") to match the
examples.
In `@skills/dagu/references/executors.md`:
- Around line 415-419: Update the merge-rule wording to reference with.fallback
instead of implying fallback is a top-level step field: change the sentence that
reads "step-level `fallback` replaces the DAG-level `fallback`" to "step-level
`with.fallback` replaces the DAG-level `with.fallback`" and ensure any related
mentions (e.g., the reserved keys list and the description of `harness`/`with`
overlay behavior) consistently use `with.fallback`; keep references to `with`,
`harness`, and `type` intact.
In `@skills/dagu/references/schema.md`:
- Line 88: Update the table row for the `with` field to state that `with` and
the legacy alias `config` are mutually exclusive: modify the description for
`with` (currently "Executor-specific config. Legacy alias: `config`") to append
a brief note such as "Cannot be used together with `config` (mutually
exclusive)". Ensure the note appears next to the legacy alias mention so readers
know the validation rejects steps that include both fields, and update any
nearby documentation text that references `config`/`with` to reflect this
mutual-exclusion rule.
---
Outside diff comments:
In `@skills/dagu/references/codingagent.md`:
- Line 68: The docs line for option_flags still uses the deprecated phrase
"config key"; update the wording in the `option_flags` entry of the `with` docs
from "config key" to "`with` key" so it reads "`option_flags` — per-option
override from `with` key to exact flag token", ensuring the documentation
consistently uses the `with` terminology.
---
Nitpick comments:
In `@internal/core/spec/step_types_test.go`:
- Around line 93-118: Add a parallel test similar to
TestCustomStepTypes_RejectWithAndLegacyConfig that exercises a handler call-site
(e.g., under handler_on.success) using both "with" and "config" together and
asserts LoadYAML returns an error containing `fields "with" and "config" cannot
be used together`; specifically create a new test (e.g.,
TestCustomStepTypes_RejectWithAndLegacyConfig_Handler) that calls LoadYAML with
YAML where a handler_on.success step invokes the custom step "greet" and
includes both a with block and a config block, then require.Error on the result
and assert the error message contains the same conflict text so the handler path
enforces the same invariant (reuse LoadYAML and the same assert.Contains pattern
used in TestCustomStepTypes_RejectWithAndLegacyConfig).
In `@internal/service/frontend/api/v1/dags_internal_test.go`:
- Around line 189-191: Add two new tests alongside the existing test that uses
the canonical with: key: one that sends the legacy config: alias as input and
asserts UpdateDAGSpec still accepts it, and another that sets both with: and
config: in the same payload and asserts UpdateDAGSpec returns a validation
error; place them in internal/service/frontend/api/v1/dags_internal_test.go near
the existing "with:" test so they exercise the same test harness and call the
same UpdateDAGSpec helper/endpoint used by the current test. Ensure the legacy
test constructs the payload using "config:" instead of "with:" and asserts
success, while the conflict test constructs a payload containing both keys and
asserts the API responds with the expected validation failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 24ccd2b3-ce12-486a-86f5-ba762edbba84
📒 Files selected for processing (38)
README.mdapi/v1/api.gen.goapi/v1/api.yamlinternal/cmn/schema/dag.schema.jsoninternal/cmn/schema/dag_schema_test.gointernal/core/spec/builder.gointernal/core/spec/builder_test.gointernal/core/spec/dag.gointernal/core/spec/kubernetes_test.gointernal/core/spec/loader_test.gointernal/core/spec/step.gointernal/core/spec/step_test.gointernal/core/spec/step_types.gointernal/core/spec/step_types_test.gointernal/intg/ct_test.gointernal/intg/distr/step_types_test.gointernal/intg/dollar_escape_test.gointernal/intg/harness_test.gointernal/intg/jq_test.gointernal/intg/redis_test.gointernal/intg/sftp_test.gointernal/intg/sql_executor_test.gointernal/intg/ssh_test.gointernal/intg/step_types_test.gointernal/intg/template_test.gointernal/runtime/builtin/agentstep/executor.gointernal/runtime/builtin/docker/executor.gointernal/runtime/builtin/kubernetes/executor.gointernal/runtime/builtin/s3/s3.gointernal/runtime/builtin/s3/s3_test.gointernal/service/frontend/api/v1/dags_internal_test.goskills/dagu/SKILL.mdskills/dagu/references/codingagent.mdskills/dagu/references/executors.mdskills/dagu/references/schema.mdui/src/api/v1/schema.tsui/src/features/dags/components/dag-editor/__tests__/customStepSchema.test.tsui/src/features/dags/components/dag-editor/customStepSchema.ts
Comment on lines
108
to
110
| - DAG-level primary harness config is the base | ||
| - Step-level `with` overlays it | ||
| - Step-level `fallback` replaces DAG-level `fallback` |
There was a problem hiding this comment.
Clarify that fallback is nested under with.
“Step-level fallback” can read like a top-level step field, but the examples place it under with.
📝 Proposed wording fix
-- Step-level `fallback` replaces DAG-level `fallback`
+- Step-level `with.fallback` replaces DAG-level `fallback`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/dagu/references/codingagent.md` around lines 108 - 110, The wording is
ambiguous about where step-level fallback belongs; clarify that the step-level
"fallback" is nested under the step's "with" block rather than being a top-level
step field: update the lines describing harness config so they read something
like "DAG-level primary harness config is the base; Step-level `with` overlays
it; Step-level `with.fallback` replaces DAG-level `fallback`", and ensure
references to `fallback` explicitly show it as `with.fallback` (or describe
"fallback under `with`") to match the examples.
Comment on lines
+415
to
+419
| The `command` field is the prompt. `with.provider` can reference either a built-in provider or a named custom entry under top-level `harnesses:`. All non-reserved `with` keys are passed directly as CLI flags (`--key value` for strings/numbers, `--key` for booleans). Built-in providers also normalize `snake_case` keys to kebab-case flag names. Reserved keys are `provider` and `fallback`. | ||
|
|
||
| Supported providers: `claude`, `codex`, `copilot`, `opencode`, `pi`. | ||
|
|
||
| Top-level `harness:` acts as a DAG-wide default. Step-level config overlays the DAG-level primary config, and step-level `fallback` replaces the DAG-level `fallback`. If a step omits `type:` and the DAG defines `harness:`, the step is inferred as `type: harness`. | ||
| Top-level `harness:` acts as a DAG-wide default. Step-level `with` overlays the DAG-level primary harness config, and step-level `fallback` replaces the DAG-level `fallback`. If a step omits `type:` and the DAG defines `harness:`, the step is inferred as `type: harness`. |
There was a problem hiding this comment.
Use with.fallback in the merge-rule wording.
This avoids implying that fallback is a top-level step field.
📝 Proposed wording fix
-Top-level `harness:` acts as a DAG-wide default. Step-level `with` overlays the DAG-level primary harness config, and step-level `fallback` replaces the DAG-level `fallback`. If a step omits `type:` and the DAG defines `harness:`, the step is inferred as `type: harness`.
+Top-level `harness:` acts as a DAG-wide default. Step-level `with` overlays the DAG-level primary harness config, and step-level `with.fallback` replaces the DAG-level `fallback`. If a step omits `type:` and the DAG defines `harness:`, the step is inferred as `type: harness`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| The `command` field is the prompt. `with.provider` can reference either a built-in provider or a named custom entry under top-level `harnesses:`. All non-reserved `with` keys are passed directly as CLI flags (`--key value` for strings/numbers, `--key` for booleans). Built-in providers also normalize `snake_case` keys to kebab-case flag names. Reserved keys are `provider` and `fallback`. | |
| Supported providers: `claude`, `codex`, `copilot`, `opencode`, `pi`. | |
| Top-level `harness:` acts as a DAG-wide default. Step-level config overlays the DAG-level primary config, and step-level `fallback` replaces the DAG-level `fallback`. If a step omits `type:` and the DAG defines `harness:`, the step is inferred as `type: harness`. | |
| Top-level `harness:` acts as a DAG-wide default. Step-level `with` overlays the DAG-level primary harness config, and step-level `fallback` replaces the DAG-level `fallback`. If a step omits `type:` and the DAG defines `harness:`, the step is inferred as `type: harness`. | |
| The `command` field is the prompt. `with.provider` can reference either a built-in provider or a named custom entry under top-level `harnesses:`. All non-reserved `with` keys are passed directly as CLI flags (`--key value` for strings/numbers, `--key` for booleans). Built-in providers also normalize `snake_case` keys to kebab-case flag names. Reserved keys are `provider` and `fallback`. | |
| Supported providers: `claude`, `codex`, `copilot`, `opencode`, `pi`. | |
| Top-level `harness:` acts as a DAG-wide default. Step-level `with` overlays the DAG-level primary harness config, and step-level `with.fallback` replaces the DAG-level `fallback`. If a step omits `type:` and the DAG defines `harness:`, the step is inferred as `type: harness`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/dagu/references/executors.md` around lines 415 - 419, Update the
merge-rule wording to reference with.fallback instead of implying fallback is a
top-level step field: change the sentence that reads "step-level `fallback`
replaces the DAG-level `fallback`" to "step-level `with.fallback` replaces the
DAG-level `with.fallback`" and ensure any related mentions (e.g., the reserved
keys list and the description of `harness`/`with` overlay behavior) consistently
use `with.fallback`; keep references to `with`, `harness`, and `type` intact.
| | `container` | string or object | — | Step-level container config | | ||
| | `type` | string | — | Executor type override | | ||
| | `config` | map | — | Executor-specific config | | ||
| | `with` | map | — | Executor-specific config. Legacy alias: `config` | |
There was a problem hiding this comment.
Document that with and config are mutually exclusive.
The new validation rejects steps that specify both fields, so the schema reference should call that out next to the legacy alias note.
Suggested wording
-| `with` | map | — | Executor-specific config. Legacy alias: `config` |
+| `with` | map | — | Executor-specific config. Legacy alias: `config`; do not specify both fields on the same step. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `with` | map | — | Executor-specific config. Legacy alias: `config` | | |
| | `with` | map | — | Executor-specific config. Legacy alias: `config`; do not specify both fields on the same step. | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/dagu/references/schema.md` at line 88, Update the table row for the
`with` field to state that `with` and the legacy alias `config` are mutually
exclusive: modify the description for `with` (currently "Executor-specific
config. Legacy alias: `config`") to append a brief note such as "Cannot be used
together with `config` (mutually exclusive)". Ensure the note appears next to
the legacy alias mention so readers know the validation rejects steps that
include both fields, and update any nearby documentation text that references
`config`/`with` to reflect this mutual-exclusion rule.
zhnq
pushed a commit
to zhnq/dagu
that referenced
this pull request
Apr 23, 2026
Merged features from upstream: - feat: add embedded Dagu API (dagucloud#2011) - feat: add edit-and-retry DAG runs (dagucloud#2010) - feat: add bulk DAG-run deletion in web UI (dagucloud#2009) - feat: add kubernetes secret provider (dagucloud#2006) - feat: make workspace selection global (dagucloud#2015) - feat: show cockpit run artifacts (dagucloud#2017) - feat: allow disabling DAG retry policy (dagucloud#2018) - feat: make DAG labels canonical, deprecate tags (dagucloud#2013) - feat: add workflow design workspace (dagucloud#2012) - feat: add step with/config alias (dagucloud#2021) - fix: allow runtime custom step inputs (dagucloud#2005) - fix: align embedded engine run labels (dagucloud#2014) - Various CI/test/docs improvements Conflict resolution: - Kept fork i18n files (ui/src/i18n/, LanguageSwitcher) - Merged i18n hooks with upstream structural changes in App.tsx, Layout.tsx, menu.tsx, DAGStatus.tsx, etc. - Adopted upstream labels-over-tags rename (TagCombobox -> LabelCombobox)
This was referenced May 6, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
withexecutor input while keeping legacyconfigas an aliaswithandconfig, including custom step type call siteswithTesting
Summary by CodeRabbit
Release Notes
Breaking Changes
withfield instead ofconfig. Theconfigfield is supported as a deprecated legacy alias but cannot be used together withwithin the same step; using both will result in a validation error.Documentation
withfield naming convention.