feat: add builtin agent harness provider by yohamta0 · Pull Request #2236 · dagucloud/dagu

yohamta0 · 2026-05-31T15:03:07Z

Summary

add provider: builtin for harness.run as in-process syntax sugar over the existing agent executor
support DAG-wide builtin harness config, mixed CLI/builtin fallbacks, chat-message persistence, and schema/docs updates
split all built-in harness providers from CLI-only providers so builtin is not treated as a binary-backed provider

Testing

go test ./internal/core ./internal/core/spec ./internal/runtime ./internal/runtime/builtin/harness ./internal/runtime/builtin/agentstep ./internal/cmn/schema ./internal/cmd -count=1
git diff --check

Summary by cubic

Adds a built-in agent provider to harness.run via provider: builtin, so you can run Dagu’s agent in-process without a CLI. Includes DAG-level defaults, mixed fallbacks, strict validation, and chat history/push-back for builtin runs; README updated and implementation notes removed.

New Features
- provider: builtin executes the agent in-process by delegating to the existing agent executor (no external binary).
- DAG-level harness config can set builtin defaults; steps inherit and can override.
- Supports mixing builtin and CLI providers in fallback with ordered execution.
- Validates builtin agent fields and dashed aliases (max-iterations, safe-mode, web-search), including nested tools.bash_policy, memory.enabled, web_search, skills, soul, model, max_iterations, safe_mode.
- Chat-message persistence and push-back are enabled for harness steps when any attempt uses builtin; cancellation handling reports canceled runs correctly.
- Built-in provider list split into CLI-only vs builtin; CLI flag normalization applies only to CLI providers.
- Docs: README wording updated; removed implementation-notes.md.
Migration
- Use provider: builtin under harness: or step with: to run the in-process agent.
- Do not define builtin under harnesses:; it’s reserved and will fail validation.
- CLI-only flags (e.g., full-auto) are rejected with builtin; use agent fields like model, tools, and max_iterations instead.

^{Written for commit 68811a5. Summary will update on new commits.}

Summary by CodeRabbit

New Features
- Added support for provider: builtin in harness configuration, enabling users to run Dagu's built-in agent directly via harness actions.
- Built-in harness provider supports configuration options including tools, safe mode, and max iterations.
- Fallback support between built-in agent and CLI harness providers.
Documentation
- Updated README to clarify harness-agnostic capability now includes running the built-in agent.
- Added implementation notes documenting built-in harness provider behavior and configuration.

coderabbitai · 2026-05-31T15:03:22Z

📝 Walkthrough

Walkthrough

This PR adds support for a new "builtin" harness provider that executes Dagu's in-process agent executor, complementing existing CLI-based harness providers (codex, etc.). It includes core provider predicates, agent config parsing, builtin execution in the harness executor, chat message wiring, step runner integration for message persistence, validation updates, comprehensive tests, and documentation changes.

Changes

Built-in Agent Harness Provider

Layer / File(s)	Summary
Builtin provider constant and predicates `internal/core/harness.go`, `internal/core/harness_test.go`	Introduces `HarnessProviderBuiltin = "builtin"` constant and splits provider detection into `IsBuiltinAgentHarnessProvider()` and `IsBuiltinCLIHarnessProvider()` helpers; updates `IsBuiltinHarnessProvider()` to check both; adds `ValidateBuiltinAgentHarnessConfig()` to validate config keys.
Agent config parsing from builtin harness `internal/runtime/builtin/harness/agent_config.go`, `internal/runtime/builtin/agentstep/executor.go`	Implements `agentConfigFromBuiltinHarnessConfig()` to parse builtin harness config into `AgentStepConfig` with validation, type conversion helpers (`intFromValue`, `stringSliceFromValue`), and nested parsers for tools, bash rules, memory, and web search settings; exports `NewExecutor()` in agentstep to construct agent executors.
Builtin agent step execution in harness `internal/runtime/builtin/harness/builtin.go`	Implements `runBuiltinOnce()` to create agent executor, manage stdout spooling, wire chat messages and pushback context, handle cancellation with mutex-protected state, and interpret outcomes (exit code 124 for cancellation); implements `builtinAgentStep()` to construct the agent step from builtin harness flags and prompt/script message.
Harness executor state, messaging, and provider routing `internal/runtime/builtin/harness/harness.go`	Extends `harnessExecutor` with builtin state (`cancelBuiltin`, `builtinStopped`, `contextMessages`, `savedMessages`); adds `SetContext()` and `GetMessages()` methods; updates `runOnce()` to dispatch to `runBuiltinOnce()` for builtin providers; updates `stop()` to cancel builtin execution.
Harness provider resolution and validation for builtin `internal/runtime/builtin/harness/harness.go`	Updates `resolveProvider()` to detect builtin agent harness and return config with `builtin: true`; updates `validateProviderConfig()` to delegate to `ValidateBuiltinAgentHarnessConfig()` for builtin provider validation.
DAG and step spec validation `internal/core/spec/dag.go`, `internal/core/spec/step.go`	Updates `validateHarnessProviderConfig()` to branch early for `IsBuiltinAgentHarnessProvider()` and call `ValidateBuiltinAgentHarnessConfig()`; updates `mergeHarnessConfig()` to use `IsBuiltinCLIHarnessProvider()` predicate for flag normalization in both step and DAG fallback paths.
Step runner integration for builtin harness chat messages `internal/runtime/runner.go`, `internal/runtime/export_test.go`	Introduces `stepSupportsChatMessages()` and `harnessConfigHasBuiltinProvider()` to detect harness steps with builtin providers across multiple fallback config shapes; updates `setupChatMessages()` and `setupPushBackConversation()` to gate on `stepSupportsChatMessages()`; exports `StepSupportsChatMessages()` test helper.
Tests covering core, harness, runner, and builder `internal/core/harness_test.go`, `internal/runtime/builtin/harness/harness_test.go`, `internal/core/spec/builder_test.go`, `internal/runtime/runner_test.go`	Adds predicate tests for `IsBuiltinAgentHarnessProvider()` and `IsBuiltinCLIHarnessProvider()`; adds harness validation/resolution/execution tests with builtin and fallback combinations; adds DAG inheritance and collision tests for builtin harness; adds runner chat-message-handler subtests for builtin configurations.
Documentation and schema updates `README.md`, `implementation-notes.md`, `internal/cmn/schema/dag.schema.json`, `internal/runtime/builtin/harness/schema.go`	Updates README Highlights and Built-in Actions table to document builtin agent capability; adds 2026-05-31 implementation notes describing contract, delegation, message persistence, and taxonomy cleanup; updates JSON schema descriptions and harness schema to clarify builtin provider support.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

dagucloud/dagu#1978: Introduces the base internal/runtime/builtin/harness executor; this PR extends it with the builtin agent provider path and message wiring.
dagucloud/dagu#2071: Both PRs update mergeHarnessConfig in spec/step.go to handle builtin provider flag normalization; main PR changes the predicate used to determine when normalization applies.
dagucloud/dagu#2109: Both PRs extend the harness/agent executor messaging interfaces and push-back plumbing; main PR adds consumer side (SetContext/GetMessages), while retrieved PR adds producer/state management on the harness side.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main feature addition: a new builtin agent harness provider.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description covers the summary, key features, migration notes, and testing approach, aligning well with the description template structure.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/builtin-agent-harness

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

internal/cmn/schema/dag.schema.json (1)
4514-4567: ⚡ Quick win

Clarify pass-through wording for provider: builtin configs.

These sections now define builtin-specific fields, but the descriptions still imply unknown/non-reserved keys are generally passed through to CLI flags. That conflicts with builtin-provider validation (allowlist-based) and can mislead users. Please scope the wording to CLI providers and explicitly note builtin keys are validated.

Also applies to: 4576-4635
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/cmn/schema/dag.schema.json` around lines 4514 - 4567, Update the
descriptions for the harness provider configuration (the object keyed by
"provider" with properties like "model", "tools", "memory", "max_iterations",
"safe_mode", "skills", "soul", "web_search" and the "additionalProperties"
referencing "harnessConfigValue") to clarify that only CLI-style providers pass
unknown/non-reserved keys through as CLI flags, and explicitly state that when
"provider" is "builtin" the listed keys are validated against the builtin
allowlist (i.e., builtin-specific fields are validated and not blindly passed
through); apply the same wording change to the equivalent block referenced
around the second occurrence noted (lines 4576-4635).
internal/runtime/builtin/harness/harness.go (1)
730-732: 💤 Low value

Consider using the predicate for consistency.

This uses direct string comparison with core.HarnessProviderBuiltin, while validateHarnessProviderConfig in dag.go uses core.IsBuiltinAgentHarnessProvider(). Using the predicate here would ensure consistent behavior if the predicate logic ever changes (e.g., trimming, case handling).
Suggested change
-	if providerStr == core.HarnessProviderBuiltin {
+	if core.IsBuiltinAgentHarnessProvider(providerStr) {
 		return core.ValidateBuiltinAgentHarnessConfig(cfg)
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/runtime/builtin/harness/harness.go` around lines 730 - 732, The code
currently compares providerStr to core.HarnessProviderBuiltin directly; update
it to use the shared predicate core.IsBuiltinAgentHarnessProvider(providerStr)
for consistency with validateHarnessProviderConfig in dag.go and to preserve any
normalization logic; if the predicate returns true, call
core.ValidateBuiltinAgentHarnessConfig(cfg) as before (keeping cfg and the same
return behavior).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/core/harness.go`:
- Around line 81-103: The validation in ValidateBuiltinAgentHarnessConfig uses
builtinAgentHarnessConfigKeys with underscore names (e.g., "max_iterations") but
keys are canonicalized to kebab-case by NormalizeBuiltinHarnessFlagKeys, causing
valid keys to be rejected; fix this by either normalizing incoming cfg keys back
to the underscore form before the lookup in ValidateBuiltinAgentHarnessConfig or
by expanding builtinAgentHarnessConfigKeys to include the kebab-case equivalents
(e.g., "max-iterations", "safe-mode", "web-search"); update the check inside
ValidateBuiltinAgentHarnessConfig to perform the same normalization/mapping used
elsewhere so both formats pass.

In `@internal/runtime/builtin/harness/builtin.go`:
- Around line 59-78: After calling agentExec.Run(runCtx) in the harness, detect
parent-context cancellation by checking runCtx.Err() (or ctx.Err()) and treat it
the same as e.builtinStopped: if runCtx.Err() == context.Canceled (or ctx.Err()
== context.Canceled) set e.exitCode = 124, call cleanupStdoutSpool(stdout) and
return nil, context.Canceled; keep existing cancel() and the e.mu protected
updates to e.cancelBuiltin/e.builtinStopped, but ensure the cancellation branch
is evaluated after Run and before falling through to a successful return so the
canceled run does not return the spool as success (refer to runCtx, ctx,
agentExec.Run, e.builtinStopped, e.exitCode, cleanupStdoutSpool).

---

Nitpick comments:
In `@internal/cmn/schema/dag.schema.json`:
- Around line 4514-4567: Update the descriptions for the harness provider
configuration (the object keyed by "provider" with properties like "model",
"tools", "memory", "max_iterations", "safe_mode", "skills", "soul", "web_search"
and the "additionalProperties" referencing "harnessConfigValue") to clarify that
only CLI-style providers pass unknown/non-reserved keys through as CLI flags,
and explicitly state that when "provider" is "builtin" the listed keys are
validated against the builtin allowlist (i.e., builtin-specific fields are
validated and not blindly passed through); apply the same wording change to the
equivalent block referenced around the second occurrence noted (lines
4576-4635).

In `@internal/runtime/builtin/harness/harness.go`:
- Around line 730-732: The code currently compares providerStr to
core.HarnessProviderBuiltin directly; update it to use the shared predicate
core.IsBuiltinAgentHarnessProvider(providerStr) for consistency with
validateHarnessProviderConfig in dag.go and to preserve any normalization logic;
if the predicate returns true, call core.ValidateBuiltinAgentHarnessConfig(cfg)
as before (keeping cfg and the same return behavior).

🪄 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: 472cb9c6-7bc9-46d8-b2c9-212eb81db023

📥 Commits

Reviewing files that changed from the base of the PR and between 683ee5b and d7165ce.

📒 Files selected for processing (17)

README.md
implementation-notes.md
internal/cmn/schema/dag.schema.json
internal/core/harness.go
internal/core/harness_test.go
internal/core/spec/builder_test.go
internal/core/spec/dag.go
internal/core/spec/step.go
internal/runtime/builtin/agentstep/executor.go
internal/runtime/builtin/harness/agent_config.go
internal/runtime/builtin/harness/builtin.go
internal/runtime/builtin/harness/harness.go
internal/runtime/builtin/harness/harness_test.go
internal/runtime/builtin/harness/schema.go
internal/runtime/export_test.go
internal/runtime/runner.go
internal/runtime/runner_test.go

cubic-dev-ai

1 issue found across 17 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic}

feat: add builtin agent harness provider

d7165ce

coderabbitai Bot reviewed May 31, 2026

View reviewed changes

Comment thread internal/core/harness.go

Comment thread internal/runtime/builtin/harness/builtin.go

cubic-dev-ai Bot reviewed May 31, 2026

View reviewed changes

Comment thread internal/runtime/builtin/harness/builtin.go Outdated

yohamta0 added 2 commits June 1, 2026 00:15

fix: address builtin harness review feedback

8cb2324

docs: remove implementation notes

68811a5

yohamta0 merged commit 2c30589 into main Jun 1, 2026
11 checks passed

yohamta0 deleted the feat/builtin-agent-harness branch June 1, 2026 04:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: add builtin agent harness provider#2236

feat: add builtin agent harness provider#2236
yohamta0 merged 3 commits into
mainfrom
feat/builtin-agent-harness

yohamta0 commented May 31, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

coderabbitai Bot commented May 31, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

yohamta0 commented May 31, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Summary by cubic

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yohamta0 commented May 31, 2026 •

edited by cubic-dev-ai Bot

Loading

coderabbitai Bot commented May 31, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading