diff --git a/internal/pipeline/executor.go b/internal/pipeline/executor.go index 3e5e1a48d..5c805d2d3 100644 --- a/internal/pipeline/executor.go +++ b/internal/pipeline/executor.go @@ -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 @@ -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 diff --git a/internal/pipeline/executor_test.go b/internal/pipeline/executor_test.go index 8064edefb..66e7d943c 100644 --- a/internal/pipeline/executor_test.go +++ b/internal/pipeline/executor_test.go @@ -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