From e079c9088def516b93d2b66e5b7ba23cbee9423f Mon Sep 17 00:00:00 2001 From: "K. Hodges" Date: Sun, 17 May 2026 13:56:36 -0700 Subject: [PATCH] The bug was that validate_patch had no on_fail, so NightShift stopped instead of sending that feedback back to implement. --- QUICKSTART.md | 1 + README.md | 1 + docs/tutorial/01-intro.md | 1 + examples/quickstart-lisp/nightshift.yaml | 1 + nightshift/agents.py | 1 + tests/test_pipeline.py | 53 ++++++++++++++++++++++++ 6 files changed, 58 insertions(+) diff --git a/QUICKSTART.md b/QUICKSTART.md index b95bbfe..dd50a94 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -172,6 +172,7 @@ pipeline: - id: validate_patch type: patch_validator output: patch-validation.md + on_fail: implement - id: apply_patch type: patch_apply diff --git a/README.md b/README.md index bc1d5a7..dba8a8c 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,7 @@ pipeline: output: patch-validation.md max_files: 8 max_lines: 800 + on_fail: implement - id: apply_patch type: patch_apply diff --git a/docs/tutorial/01-intro.md b/docs/tutorial/01-intro.md index 57d4d0c..b40b8a3 100644 --- a/docs/tutorial/01-intro.md +++ b/docs/tutorial/01-intro.md @@ -251,6 +251,7 @@ Patch validator: output: patch-validation.md max_files: 4 max_lines: 400 + on_fail: implement forbidden_paths: - .git - .nightshift diff --git a/examples/quickstart-lisp/nightshift.yaml b/examples/quickstart-lisp/nightshift.yaml index 618456e..29ef7db 100644 --- a/examples/quickstart-lisp/nightshift.yaml +++ b/examples/quickstart-lisp/nightshift.yaml @@ -62,6 +62,7 @@ pipeline: output: patch-validation.md max_files: 4 max_lines: 400 + on_fail: implement - id: apply_patch type: patch_apply diff --git a/nightshift/agents.py b/nightshift/agents.py index 1bf5652..e67f1a1 100644 --- a/nightshift/agents.py +++ b/nightshift/agents.py @@ -468,6 +468,7 @@ def output_contract_for(stage: StageConfig) -> str: "Return a unified diff only, suitable for saving as proposed.patch.", "Do not include prose outside the patch.", "Use diff --git headers and hunk headers.", + "For existing files, do not use new file mode or /dev/null headers.", ] ) if stage.type == "patch_normalizer": diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index db0da85..f1d36bd 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -454,6 +454,59 @@ Acceptance Criteria: self.assertEqual(result.status, "failed") self.assertIn("forbidden path", result.reason) + def test_patch_validation_failure_can_retry_implementation(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + _write_common_files(root) + (root / "app.py").write_text("old\n", encoding="utf-8") + (root / "fake_writer.py").write_text( + "\n".join( + [ + "import sys", + "prompt = sys.stdin.read()", + "new_file_patch = 'Retry 1:' not in prompt", + "if new_file_patch:", + " print('diff --git a/app.py b/app.py')", + " print('new file mode 100644')", + " print('--- /dev/null')", + " print('+++ b/app.py')", + " print('@@ -0,0 +1 @@')", + " print('+bad')", + "else:", + " print('diff --git a/app.py b/app.py')", + " print('--- a/app.py')", + " print('+++ b/app.py')", + " print('@@ -1 +1 @@')", + " print('-old')", + " print('+new')", + ] + ), + encoding="utf-8", + ) + stages = ( + StageConfig(id="write", type="code_writer", agent="writer"), + StageConfig(id="normalize", type="patch_normalizer"), + StageConfig(id="validate", type="patch_validator", on_fail="write"), + ) + config = make_config(root, stages, max_retries=1) + config.agents["writer"] = AgentConfig( + id="writer", + backend="command", + command="python fake_writer.py", + system_prompt=Path("planner.md"), + ) + runner = PipelineRunner(config, ArtifactStore(root, ".nightshift", run_id="test-run")) + + result = runner.run_task(parse_tasks(TASK_MD)[0]) + + task_dir = root / ".nightshift" / "runs" / "test-run" / "tasks" / "TASK-001" + self.assertEqual(result.status, "complete") + self.assertEqual(result.retry_count, 1) + self.assertTrue( + any("creates existing file" in stage.reason for stage in result.stage_results) + ) + self.assertTrue((task_dir / "repair-1.patch").exists()) + def test_patch_apply_stage_applies_patch(self) -> None: with tempfile.TemporaryDirectory() as directory: root = Path(directory)