Skip to content

fix(pipeline): register command-step output_artifacts for downstream injection#1491

Merged
nextlevelshit merged 1 commit into
mainfrom
fix/1490-command-output-artifacts
Apr 28, 2026
Merged

fix(pipeline): register command-step output_artifacts for downstream injection#1491
nextlevelshit merged 1 commit into
mainfrom
fix/1490-command-output-artifacts

Conversation

@nextlevelshit
Copy link
Copy Markdown
Collaborator

Closes #1490.

Problem

type: command steps that wrote files declared in output_artifacts never populated execution.ArtifactPaths[step.ID+\":\"+art.Name]. The persona/adapter dispatch path called writeOutputArtifacts after the adapter returned (executor.go:3572-3583); the command dispatch path returned directly from executeCommandStep to validateStepContracts without ever registering the path.

Downstream memory.inject_artifacts lookup (executor.go:4555) missed on every command-step source, fell through to the stdout-fallback branch, and (since command scripts almost never echo the artifact bytes to stdout) wrote a 0-byte file under .agents/artifacts/<as-name>. The receiving persona then read empty input and short-circuited. The pipeline reported success.

Real-world hit

ops-pr-respond 1472 (run ops-pr-respond-20260428-182026-1f67) posted "No findings" to PR #1472 — even though filter-scope (a command step) had produced 15 valid security findings. The planner persona received empty scoped_findings. Pipeline status: green. Deliverable: wrong.

This is exactly the failure mode "green ≠ correct" — masked from CI, masked from web UI, only caught by reading the actual JSON outputs after the run.

Fix

Call writeOutputArtifacts(execution, step, workspacePath, nil) after executeCommandStep returns successfully on both dispatch paths:

  • executeStep at ~1786 (the linear walker)
  • the executeGraphPipeline callback at ~1318 (the DAG walker)

writeOutputArtifacts already handles the "script wrote the file, don't overwrite" case via os.Stat(artPath) == nil (line 4715) — for command steps this is always the branch taken.

Test

TestCommandStepOutputArtifactsRegisteredForInjection reproduces the bug shape: a command step writes JSON to a declared output_artifact, then a downstream persona consumes it via inject_artifacts. Without the executor.go change the injected file is 0 bytes; with the change it contains the JSON the command produced.

I confirmed locally: git stash the executor.go fix → test fails ("0 is not greater than 0"). Restore → passes.

Test plan

  • go test ./internal/pipeline/... — green
  • New regression test passes with fix, fails without
  • Re-run ops-pr-respond 1472 after merge — expect substantive PR comment listing real findings

…injection

Closes #1490.

Problem
=======

`type: command` steps that wrote files declared in `output_artifacts`
never populated `execution.ArtifactPaths[step.ID+":"+art.Name]`. The
persona/adapter dispatch path called `writeOutputArtifacts` after the
adapter returned (executor.go:3572-3583); the command dispatch path
returned directly from `executeCommandStep` to `validateStepContracts`
without ever registering the path.

Downstream `memory.inject_artifacts` lookup (executor.go:4555) missed
on every command-step source, fell through to the stdout-fallback
branch, and (since command scripts almost never echo the artifact bytes
to stdout) wrote a **0-byte file** under `.agents/artifacts/<as-name>`.

The receiving persona then read empty input and short-circuited. The
pipeline reported success.

Real-world hit: `ops-pr-respond 1472` posted "No findings" to the PR
even though `filter-scope` had produced 15 valid security findings —
because the planner persona received empty `scoped_findings`.

Fix
===

Call `writeOutputArtifacts(execution, step, workspacePath, nil)` after
`executeCommandStep` returns successfully on both dispatch paths
(`executeStep` at ~1786, and the `executeGraphPipeline` callback at
~1318). The function already handles the "script wrote the file, don't
overwrite" case via `os.Stat(artPath) == nil` (line 4715) — for
command steps this is always the path taken.

Test
====

`TestCommandStepOutputArtifactsRegisteredForInjection` reproduces the
bug shape: a command step writes JSON to a declared output_artifact,
then a downstream persona consumes it via `inject_artifacts`. Without
the fix the injected file is 0 bytes; with the fix it contains the
JSON the command produced. Verified failing without the executor.go
edit and passing with it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pipeline ships wrong deliverable when command-step output_artifacts feed downstream inject_artifacts

1 participant