fix(workflow): pass output validation on resume-after-approval (#518) - #531
Conversation
Test-TaskOutput counted only files produced during the current run (delta vs baseline). On resume-after-approval the worktree already holds the artifact from the prior run, so the agent calls task_set_status(done) without re-writing files — delta is 0 and validation failed with "produced 0 new file(s)", escalating a correctly-completed task to needs-input. For non-tasks/ outputs, fall back to the absolute file count when the delta is below min_output_count: if the required files already exist, pass. tasks/ outputs keep strict delta enforcement because manifest pre-creation makes the absolute count always look satisfied. Fixes andresharpe#518
|
@kabaogluemre I think this PR is fixing a real symptom, but I’d like us to take a different direction here. The current issue comes from dotbot trying to infer task success by counting files. That may have made sense when every workflow step was expected to produce local artifacts, but I don’t think we should carry that model forward. dotbot should be able to orchestrate workflows where a step succeeds by calling an API, updating an external system, triggering another process, or doing something that does not create files at all. Rather than making the file-counting logic more nuanced, I think we should remove that implicit validation from dotbot and move validation into explicit workflow steps where needed. Each workflow can decide what success means for that domain. For example, a workflow that cares about generated planning tasks could make validation explicit: {
"name": "Create Planning Tasks",
"type": "prompt",
"workflow": "02-plan-tasks.md"
},
{
"name": "Validate Planning Tasks",
"type": "script",
"script": "Scripts/Validate-PlanningTasks.ps1",
"depends_on": ["Create Planning Tasks"]
}That keeps dotbot focused on orchestration, and keeps success criteria owned by the workflow. In short: I don’t think we should enhance the file-counting logic here. I’d prefer we remove it and rely on explicit workflow validation steps instead. |
|
@carlospedreira agree with the direction. This PR is a scoped fix for the #518, what you're proposing is a bigger change; removing implicit validation touches all shipped workflows, and the worktree/merge timing for separate validation steps needs investigation before it's implementation-ready. |
|
@kabaogluemre agreed. I opened #555 to track the broader architectural move away from implicit runtime file-count validation and toward explicit workflow-owned validation steps. I’m okay landing this PR as the scoped stopgap for #518 while that larger design is handled separately. |
PR andresharpe#531 (fix andresharpe#518) landed Issue D upstream -- identical logic to our fix, comment wording differs only. Resolved trivial comment conflict in Invoke-WorkflowProcess.ps1 by taking upstream s wording. Issue D now fully covered by upstream. Remaining unique change: B (interview-answers.json). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Fixes #518 — output delta validation incorrectly failed on resume-after-approval when the artifact already existed in the worktree from the prior run.
Test-TaskOutputvalidates by counting new files produced during the current run (delta vs. a baseline captured before the agent starts). On a resumed run after an approval gate, the worktree already holds the artifact, so the agent correctly callstask_set_status(done)without re-writing existing files — the delta is0. Validation then failed withproduced 0 new file(s), expected at least 1, escalating a correctly-completed task toneeds-input.Fix
In
Test-TaskOutput(Invoke-WorkflowProcess.ps1): when the delta is belowmin_output_count, non-tasks/outputs fall back to the absolute file count. If the required files already exist (absolute count ≥ min), validation passes.tasks/outputs keep strict delta enforcement, because manifest pre-creation means the absolute count always looks satisfied there — leaving delta as the only meaningful signal.Behaviour
tasks/?tasks/output on resume