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
15 changes: 13 additions & 2 deletions internal/pipeline/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1320,10 +1320,14 @@ func (e *DefaultPipelineExecutor) executeGraphPipeline(ctx context.Context, p *P
if err != nil {
return result, err
}
// Register output artifacts in ArtifactPaths so downstream
// inject_artifacts can find the files the script wrote (#1490).
workspacePath := execution.WorkspacePaths[step.ID]
e.writeOutputArtifacts(execution, step, workspacePath, nil)
// Run handover contract validation for command steps.
// Command steps run in the project root (or mount target), so resolve
// contract sources against the command's actual working directory.
contractDir := resolveCommandWorkDir(execution.WorkspacePaths[step.ID], step)
contractDir := resolveCommandWorkDir(workspacePath, step)
adapterResult := &adapter.AdapterResult{}
if cErr := e.validateStepContracts(ctx, execution, step, contractDir, nil, execution.Status.ID, "", time.Now(), adapterResult); cErr != nil {
return result, cErr
Expand Down Expand Up @@ -1791,9 +1795,16 @@ func (e *DefaultPipelineExecutor) executeStep(ctx context.Context, execution *Pi
if result != nil && result.Outcome == "failure" {
return result.Error
}
// Register output artifacts so downstream `inject_artifacts` lookups
// in `injectArtifacts` (executor.go: ArtifactPaths[step.ID+":"+name])
// resolve to the on-disk file the script wrote. Without this,
// command-step outputs were silently delivered as 0-byte blobs to
// downstream personas — see #1490.
workspacePath := execution.WorkspacePaths[step.ID]
e.writeOutputArtifacts(execution, step, workspacePath, nil)
// Run handover contract validation (same as persona steps).
// Resolve against the command's actual working directory, not the workspace root.
contractDir := resolveCommandWorkDir(execution.WorkspacePaths[step.ID], step)
contractDir := resolveCommandWorkDir(workspacePath, step)
adapterResult := &adapter.AdapterResult{}
if cErr := e.validateStepContracts(ctx, execution, step, contractDir, nil, pipelineID, "", time.Now(), adapterResult); cErr != nil {
return cErr
Expand Down
77 changes: 77 additions & 0 deletions internal/pipeline/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,83 @@ func TestWriteOutputArtifactsPreservesExistingFiles(t *testing.T) {
"Persona-written artifact should be preserved when file already exists")
}

// TestCommandStepOutputArtifactsRegisteredForInjection is a regression test for
// #1490. A `type: command` step that writes a file declared in
// `output_artifacts` must register the file path in
// `execution.ArtifactPaths[step.ID+":"+art.Name]` so a downstream step's
// `memory.inject_artifacts` lookup resolves to actual content rather than
// silently falling through to the (usually empty) stdout fallback.
func TestCommandStepOutputArtifactsRegisteredForInjection(t *testing.T) {
tmpDir := t.TempDir()

mockAdapter := adapter.NewMockAdapter(
adapter.WithStdoutJSON(`{"type": "result", "result": "ok"}`),
adapter.WithTokensUsed(10),
)
collector := testutil.NewEventCollector()
executor := NewDefaultPipelineExecutor(mockAdapter, WithEmitter(collector))

m := testutil.CreateTestManifest(tmpDir)

p := &Pipeline{
Metadata: PipelineMetadata{Name: "command-output-artifact-test"},
Steps: []Step{
{
ID: "produce",
Type: StepTypeCommand,
Script: `mkdir -p .agents/output && printf '{"items":[{"x":1}]}' > .agents/output/data.json`,
OutputArtifacts: []ArtifactDef{
{Name: "data", Path: ".agents/output/data.json", Type: "json"},
},
},
{
ID: "consume",
Persona: "navigator",
Dependencies: []string{"produce"},
Memory: MemoryConfig{
InjectArtifacts: []ArtifactRef{
{Step: "produce", Artifact: "data", As: "data"},
},
},
Exec: ExecConfig{Source: "consume artifact"},
},
},
}

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

err := executor.Execute(ctx, p, m, "workspace-cmd-art")
require.NoError(t, err)

// The injected artifact should be the JSON the command wrote, not a
// 0-byte file from the stdout fallback path. Walk the tmpDir for the
// `data` injection target — its exact location depends on the
// workspace-creation strategy in effect.
var injectedPath string
walkErr := filepath.Walk(tmpDir, func(path string, info os.FileInfo, err error) error {
if err != nil || info.IsDir() {
return nil
}
if filepath.Base(path) == "data" && strings.Contains(path, filepath.Join("consume", ".agents", "artifacts")) {
injectedPath = path
}
return nil
})
require.NoError(t, walkErr)
require.NotEmpty(t, injectedPath, "injected artifact must exist somewhere under %s", tmpDir)

stat, err := os.Stat(injectedPath)
require.NoError(t, err)
assert.Greater(t, stat.Size(), int64(0),
"injected artifact must be non-empty — see #1490")

content, err := os.ReadFile(injectedPath)
require.NoError(t, err)
assert.Contains(t, string(content), `"items"`,
"injected content must match what the command wrote, not stdout fallback")
}

// configCapturingAdapter wraps MockAdapter and captures the AdapterRunConfig passed to Run
type configCapturingAdapter struct {
*adapter.MockAdapter
Expand Down
Loading