Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,399 changes: 701 additions & 698 deletions api/v1/api.gen.go

Large diffs are not rendered by default.

7 changes: 6 additions & 1 deletion api/v1/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10916,7 +10916,7 @@ components:

HumanTaskConfig:
type: object
description: "Resolved human-task instructions and optional normalized input form"
description: "Resolved human-task instructions with optional normalized input form and artifact references"
properties:
prompt:
type: string
Expand All @@ -10925,6 +10925,11 @@ components:
type: object
additionalProperties: true
description: "Normalized flat JSON Schema for typed completion input"
artifacts:
type: array
items:
type: string
description: "Artifact-relative paths in the current root DAG run"
required:
- prompt

Expand Down
32 changes: 32 additions & 0 deletions conformance/spec031_human_task/lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,35 @@ func runConcurrentCompletions(
wg.Wait()
return results
}

func TestArtifactReferencesResolveWhenTaskOpens(t *testing.T) {
dagu := harness.NewRunner(t)
env := sharedEnv(t)
const runID = "spec031-artifacts"

startWaiting(t, dagu, env, runID, "artifact_snapshot.yaml")
status := waitForStatus(t, dagu, env, runID, "artifact_snapshot.yaml", "Waiting")
require.Contains(t, status.Stdout(), "reports/production/summary.txt")
require.Contains(t, status.Stdout(), "reports/release.diff")
require.NotContains(t, status.Stdout(), "${params.target}")

// The run writes none of the referenced artifacts, and completion must not
// care.
result := complete(t, dagu, env, runID, "review", "artifact_snapshot.yaml")
result.ExpectExitCode(0)
waitForStatus(t, dagu, env, runID, "artifact_snapshot.yaml", "Succeeded")
}

func TestArtifactReferenceEscapingArtifactDirFailsWithoutOpening(t *testing.T) {
dagu := harness.NewRunner(t)
env := sharedEnv(t)
const runID = "spec031-artifact-escape"

start := dagu.RunWithEnv(env, "start", "--run-id="+runID,
"--params=target=../../secret", "artifact_escape.yaml")
start.ExpectNonZeroExitCode()
start.ExpectStderrContains("human task artifacts", "parent directory segments")

status := waitForStatus(t, dagu, env, runID, "artifact_escape.yaml", "Failed")
require.NotContains(t, status.Stdout(), "Waiting")
}
13 changes: 13 additions & 0 deletions conformance/spec031_human_task/testdata/artifact_escape.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: spec031_artifact_escape
params:
- name: target
type: string
default: reports
working_dir: .
steps:
- id: review
action: human.task
with:
prompt: Review the reports
artifacts:
- "${params.target}/summary.txt"
20 changes: 20 additions & 0 deletions conformance/spec031_human_task/testdata/artifact_snapshot.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: spec031_artifact_snapshot
params:
- name: target
type: string
default: production
working_dir: .
steps:
- id: describe
run: printf 'subject=release\n' >> "$DAGU_OUTPUT_FILE"
outputs:
- name: subject

- id: review
depends: describe
action: human.task
with:
prompt: Review the reports
artifacts:
- "reports/${params.target}/summary.txt"
- "reports/${steps.describe.outputs.subject}.diff"
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
steps:
- id: review
action: human.task
with:
prompt: Review the generated reports
artifacts:
- changes.diff
- 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
steps:
- id: review
action: human.task
with:
prompt: Review the generated reports
artifacts:
- ../../secret
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
steps:
- id: review
action: human.task
with:
prompt: Review the generated reports
artifacts:
- "${params.OUT}/../secret"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
steps:
- id: review
action: human.task
with:
prompt: Review the generated reports
artifacts: changes.diff
8 changes: 8 additions & 0 deletions conformance/spec031_human_task/testdata/valid_artifacts.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
steps:
- id: review
action: human.task
with:
prompt: Review the generated reports
artifacts:
- changes.diff
- reports/test-report.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
params:
- name: OUT
type: string
default: reports
steps:
- id: review
action: human.task
with:
prompt: Review the generated reports
artifacts:
- "${params.OUT}/report.html"
- changes.diff
6 changes: 6 additions & 0 deletions conformance/spec031_human_task/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ func TestHumanTaskShapeValidation(t *testing.T) {
valid := []string{
"valid_acknowledgement_shape.yaml",
"valid_form_shape.yaml",
"valid_artifacts.yaml",
"valid_artifacts_reference.yaml",
"child_human.yaml",
"multi_document_human.yaml",
}
Expand All @@ -34,6 +36,10 @@ func TestHumanTaskShapeValidation(t *testing.T) {
{file: "invalid_missing_id.yaml", parts: []string{"id", "explicit"}},
{file: "invalid_prompt.yaml", parts: []string{"with.prompt", "non-empty string"}},
{file: "invalid_with_field.yaml", parts: []string{"with.evidence"}},
{file: "invalid_artifacts_type.yaml", parts: []string{"with.artifacts", "array"}},
{file: "invalid_artifacts_entry.yaml", parts: []string{"with.artifacts", "strings"}},
{file: "invalid_artifacts_path.yaml", parts: []string{"with.artifacts", "parent directory"}},
{file: "invalid_artifacts_reference_path.yaml", parts: []string{"with.artifacts", "parent directory"}},
{file: "invalid_form_null.yaml", parts: []string{"with.form", "object"}},
{file: "invalid_form_root.yaml", parts: []string{"form", "type"}},
{file: "invalid_form_property.yaml", parts: []string{"1invalid", "property"}},
Expand Down
13 changes: 13 additions & 0 deletions internal/cmn/schema/dag.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -6580,6 +6580,19 @@
"form": {
"$ref": "#/definitions/humanTaskFormSchema",
"description": "Optional inline JSON Schema for typed completion input. Omit for acknowledgement-only tasks."
},
"artifacts": {
"type": "array",
"uniqueItems": true,
"items": {
"type": "string",
"minLength": 1,
"pattern": "\\S",
"not": {
"pattern": "^\\s*(?:[/\\\\]|~\\s*$|~[/\\\\]|[A-Za-z]:|\\.\\s*$|\\.[/\\\\]*\\s*$)|(?:^|[/\\\\])\\.\\.(?:[/\\\\]|\\s*$)"
}
},
"description": "Relative artifact paths shown to the operator as review context. Each path must stay within the DAG-run artifact directory."
}
},
"description": "Configuration for a processless human task in a root DAG. Root DAGs containing human tasks may run locally or on distributed workers; human tasks are not supported in sub-DAGs."
Expand Down
77 changes: 77 additions & 0 deletions internal/cmn/schema/dag_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,83 @@ steps:
properties:
nested:
type: object
`,
},
{
name: "ArtifactsList",
spec: `
steps:
- id: review
action: human.task
with:
prompt: Review the generated reports
artifacts:
- changes.diff
- reports/test-report.html
`,
valid: true,
},
{
name: "RejectArtifactsNonString",
spec: `
steps:
- id: review
action: human.task
with:
prompt: Review
artifacts:
- changes.diff
- 1
`,
},
{
name: "RejectAbsoluteArtifactPath",
spec: `
steps:
- id: review
action: human.task
with:
prompt: Review
artifacts:
- /etc/passwd
`,
},
{
name: "RejectArtifactParentSegment",
spec: `
steps:
- id: review
action: human.task
with:
prompt: Review
artifacts:
- reports/../../secret
`,
},
{
name: "AllowsUnresolvedReferenceArtifact",
spec: `
steps:
- id: review
action: human.task
with:
prompt: Review
artifacts:
- "${params.OUT}/report.html"
`,
valid: true,
},
{
name: "RejectArtifactsDuplicate",
spec: `
steps:
- id: review
action: human.task
with:
prompt: Review
artifacts:
- changes.diff
- changes.diff
`,
},
}
Expand Down
7 changes: 4 additions & 3 deletions internal/ir/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,11 @@ type ApprovalConfig struct {
RewindTo string `json:"rewindTo,omitempty"`
}

// HumanTaskConfig defines the prompt and input form for a human task step.
// HumanTaskConfig defines the prompt, input form, and artifact references for a human task step.
type HumanTaskConfig struct {
Prompt string `json:"prompt,omitempty"`
Form json.RawMessage `json:"form,omitempty"`
Prompt string `json:"prompt,omitempty"`
Form json.RawMessage `json:"form,omitempty"`
Artifacts []string `json:"artifacts,omitempty"`
}

const (
Expand Down
3 changes: 3 additions & 0 deletions internal/output/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ func (r *Renderer) renderHumanTask(node *ir.Node, isLastSection bool, prefix str
"step id: " + node.Step.ID,
"prompt: " + node.Step.HumanTask.Prompt,
}
if len(node.Step.HumanTask.Artifacts) > 0 {
details = append(details, "artifacts: "+strings.Join(node.Step.HumanTask.Artifacts, ", "))
}
if len(node.Step.HumanTask.Form) > 0 {
details = append(details, "form: "+string(node.Step.HumanTask.Form))
}
Expand Down
1 change: 1 addition & 0 deletions internal/runtime/agent/status_masking.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func maskStepSecrets(masker *masking.Masker, step ir.Step) ir.Step {
if step.HumanTask != nil {
humanTask := *step.HumanTask
humanTask.Prompt = masker.MaskString(humanTask.Prompt)
humanTask.Artifacts = maskStrings(masker, humanTask.Artifacts)
step.HumanTask = &humanTask
}

Expand Down
10 changes: 8 additions & 2 deletions internal/runtime/agent/status_masking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,27 @@ import (
"github.com/stretchr/testify/require"
)

func TestMaskNodeSecretsMasksHumanTaskPrompt(t *testing.T) {
// Artifact paths carry runtime-resolved text just like the prompt, so a secret
// reaches persisted status through either field.
func TestMaskNodeSecretsMasksHumanTask(t *testing.T) {
t.Parallel()

masker := newStatusSecretMasker([]string{"DEPLOY_TOKEN=very-secret-token"})
require.NotNil(t, masker)
node := &ir.Node{
Step: ir.Step{HumanTask: &ir.HumanTaskConfig{
Prompt: "Review very-secret-token",
Prompt: "Review very-secret-token",
Artifacts: []string{"reports/very-secret-token.html", "changes.diff"},
}},
}

maskNodeSecrets(masker, node)

require.NotNil(t, node.Step.HumanTask)
assert.NotContains(t, node.Step.HumanTask.Prompt, "very-secret-token")
require.Len(t, node.Step.HumanTask.Artifacts, 2)
assert.NotContains(t, node.Step.HumanTask.Artifacts[0], "very-secret-token")
assert.Equal(t, "changes.diff", node.Step.HumanTask.Artifacts[1])
}

func TestMaskNodeSecretsMasksStatusDetailLabels(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion internal/runtime/agent_loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ func (r *Runner) askUser(
}

logger.Info(ctx, "Agent is asking a question", slog.String("question", question))
node.OpenHumanTask(question, time.Now())
node.OpenHumanTask(question, nil, time.Now())
state.RecordEvent(agentloop.Event{
Kind: agentloop.EventAskUser,
Name: ir.AskUserStepName,
Expand Down
32 changes: 32 additions & 0 deletions internal/runtime/artifact_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"

"github.com/dagucloud/dagu/v2/internal/cmn/runenv"
cmnvalue "github.com/dagucloud/dagu/v2/internal/cmn/value"
)

func artifactOutputFilePath(ctx context.Context, raw string) (string, error) {
Expand Down Expand Up @@ -144,3 +145,34 @@ func pathInsideOrSame(parent, child string) bool {
}
return rel != ".." && !filepath.IsAbs(rel) && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
}

// resolveHumanTaskArtifacts value-resolves each authored human-task artifact
// path and re-checks the result against the artifact-relative path rules. A
// reference can introduce path segments the authored literal never had, so the
// resolved value cannot inherit the build-time check. Entries that resolve to
// the same path contribute one reference, keeping the first authored position.
func resolveHumanTaskArtifacts(ctx context.Context, artifacts []string) ([]string, error) {
if len(artifacts) == 0 {
return nil, nil
}

resolved := make([]string, 0, len(artifacts))
seen := make(map[string]struct{}, len(artifacts))
for i, raw := range artifacts {
field := cmnvalue.StepArtifactOutputField(fmt.Sprintf("with.artifacts[%d]", i))
value, err := resolveRuntimeString(ctx, raw, field)
if err != nil {
return nil, fmt.Errorf("artifact %q: %w", raw, err)
}
clean, err := cleanArtifactOutputPath(value)
if err != nil {
return nil, fmt.Errorf("artifact %q: %w", raw, err)
}
if _, exists := seen[clean]; exists {
continue
}
seen[clean] = struct{}{}
resolved = append(resolved, clean)
}
return resolved, nil
}
Loading
Loading