From ec9181eb64576c9490c79bf1f8f9f6d9635dcb57 Mon Sep 17 00:00:00 2001 From: "K. Hodges" Date: Sun, 17 May 2026 13:46:26 -0700 Subject: [PATCH] Fixes around test run 2 --- .gitignore | 1 + nightshift/git.py | 16 +++++++++++++- nightshift/pipeline.py | 46 +++++++++++++++++++++++++++++++++++----- nightshift/repo_tools.py | 2 +- tests/test_git.py | 11 ++++++++++ tests/test_pipeline.py | 46 ++++++++++++++++++++++++++++++++++++++++ tests/test_repo_tools.py | 2 +- 7 files changed, 116 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 19c24e3..afb3615 100644 --- a/.gitignore +++ b/.gitignore @@ -50,6 +50,7 @@ coverage.xml .hypothesis/ .pytest_cache/ cover/ +tiny-lisp-nightshift/ # Translations *.mo diff --git a/nightshift/git.py b/nightshift/git.py index 2831e5d..5525598 100644 --- a/nightshift/git.py +++ b/nightshift/git.py @@ -43,6 +43,15 @@ def get_git_status(project_root: Path) -> GitCommandResult: return run_git(project_root, ["status", "--short"]) +def get_git_repository_state(project_root: Path) -> GitCommandResult: + return run_git(project_root, ["rev-parse", "--is-inside-work-tree"]) + + +def is_git_repository(project_root: Path) -> bool: + state = get_git_repository_state(project_root) + return state.available and state.stdout.strip() == "true" + + def ensure_clean_worktree(project_root: Path, require_clean: bool) -> None: if not require_clean: return @@ -63,9 +72,14 @@ def write_git_artifacts(artifacts: ArtifactStore, task_id: str, when: str) -> Pa def write_diff_artifact(artifacts: ArtifactStore, task_id: str) -> Path: + if not is_git_repository(artifacts.project_root): + content = "Git diff unavailable.\n\nReason: project root is not a git work tree.\n" + return artifacts.write_stage_output(task_id, "diff.patch", content) + diff = run_git(artifacts.project_root, ["diff", "--binary"], timeout_seconds=30) if not diff.available: - content = "Git diff unavailable.\n\n" + (diff.stderr or "") + details = (diff.stderr or "unknown git error").strip() + content = f"Git diff unavailable.\n\nReason: {details}\n" elif diff.stdout: content = diff.stdout else: diff --git a/nightshift/pipeline.py b/nightshift/pipeline.py index 78774c8..63d6e6c 100644 --- a/nightshift/pipeline.py +++ b/nightshift/pipeline.py @@ -436,17 +436,49 @@ class PipelineRunner: chart_path = self.artifacts.project_context_chart_path if chart_path.exists(): enriched_outputs["project-context-chart.md"] = chart_path.read_text(encoding="utf-8", errors="replace") + context = self.context.read_context(task, retry_notes) result = self.agent_executor.run_stage( stage, task, enriched_outputs, retry_notes, - project_context=self.context.read_context(task, retry_notes).project_context, - task_context=self.context.read_context(task, retry_notes).task_context, - retry_context=self.context.read_context(task, retry_notes).retry_context, + project_context=context.project_context, + task_context=context.task_context, + retry_context=context.retry_context, ) raw_output = self._read_output(result.output_path) stdout = extract_agent_stdout(raw_output) + lookup_requests = parse_lookup_requests(stdout) + if lookup_requests and "diff --git " not in stdout: + lookup_context = self.repo_tools.execute_requests( + task.id, + lookup_requests, + filename="implementation-files-inspected.md", + ) + self.logger.event( + "agent.rerun", + "Re-running code writer with repo lookup context", + stage_id=stage.id, + task_id=task.id, + lookup_count=len(lookup_requests), + ) + rerun_outputs = dict(enriched_outputs) + rerun_outputs["repo_lookup_results"] = lookup_context + rerun_notes = [ + *retry_notes, + "Repository lookup results have been provided. Return the unified diff now; do not request more lookups.", + ] + result = self.agent_executor.run_stage( + stage, + task, + rerun_outputs, + rerun_notes, + project_context=context.project_context, + task_context=context.task_context, + retry_context="\n".join(f"- {note}" for note in rerun_notes), + ) + raw_output = self._read_output(result.output_path) + stdout = extract_agent_stdout(raw_output) try: patch = extract_unified_diff(stdout) except PipelineError as exc: @@ -657,14 +689,18 @@ class PipelineRunner: ) rerun_outputs = dict(previous_outputs) rerun_outputs["repo_lookup_results"] = lookup_context + rerun_retry_notes = [ + *retry_notes, + "Repository lookup results have been provided. Write the final plan now; do not request more lookups.", + ] rerun_result = self.agent_executor.run_stage( stage, task, rerun_outputs, - retry_notes, + rerun_retry_notes, project_context=project_context, task_context=task_context, - retry_context=retry_context, + retry_context="\n".join(f"- {note}" for note in rerun_retry_notes), ) return StageResult( stage_id=rerun_result.stage_id, diff --git a/nightshift/repo_tools.py b/nightshift/repo_tools.py index 963bf02..c34ab6e 100644 --- a/nightshift/repo_tools.py +++ b/nightshift/repo_tools.py @@ -217,7 +217,7 @@ def parse_lookup_requests(text: str, max_requests: int = DEFAULT_MAX_LOOKUP_REQU continue key, value = stripped.split(":", 1) key = key.strip() - value = value.strip().strip('"').strip("'") + value = value.strip().strip('"').strip("'").strip("`") if key == "tool" and current: flush() current[key] = value diff --git a/tests/test_git.py b/tests/test_git.py index 15d2686..6df51b8 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -38,6 +38,17 @@ class GitSafetyTests(unittest.TestCase): self.assertTrue(diff_path.exists()) self.assertIn("Git Status before", status_path.read_text(encoding="utf-8")) + def test_diff_artifact_is_concise_outside_git_repo(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + artifacts = ArtifactStore(root, ".nightshift", run_id="test-run") + + diff_path = write_diff_artifact(artifacts, "TASK-001") + + content = diff_path.read_text(encoding="utf-8") + self.assertIn("project root is not a git work tree", content) + self.assertNotIn("usage: git diff", content) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 1526e78..db0da85 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -373,6 +373,52 @@ Acceptance Criteria: self.assertTrue((task_dir / "normalized.patch").exists()) self.assertIn("Status: pass", (task_dir / "patch-validation.md").read_text(encoding="utf-8")) + def test_code_writer_lookup_requests_are_rerun_with_context(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()", + "if 'repo_lookup_results' in prompt:", + " print('diff --git a/app.py b/app.py')", + " print('--- a/app.py')", + " print('+++ b/app.py')", + " print('@@ -1 +1 @@')", + " print('-old')", + " print('+new')", + "else:", + " print('lookup_requests:')", + " print('- tool: read_file')", + " print(' path: app.py')", + ] + ), + encoding="utf-8", + ) + stages = ( + StageConfig(id="write", type="code_writer", agent="writer"), + StageConfig(id="normalize", type="patch_normalizer"), + StageConfig(id="validate", type="patch_validator"), + ) + config = make_config(root, stages) + 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.assertTrue((task_dir / "implementation-files-inspected.md").exists()) + self.assertIn("diff --git a/app.py b/app.py", (task_dir / "proposed.patch").read_text(encoding="utf-8")) + def test_patch_validator_rejects_unsafe_patch(self) -> None: with tempfile.TemporaryDirectory() as directory: root = Path(directory) diff --git a/tests/test_repo_tools.py b/tests/test_repo_tools.py index 055f1a4..f5e0116 100644 --- a/tests/test_repo_tools.py +++ b/tests/test_repo_tools.py @@ -33,7 +33,7 @@ lookup_requests: path: nightshift/pipeline.py - tool: grep path: nightshift - pattern: PipelineRunner + pattern: `PipelineRunner` """ requests = parse_lookup_requests(output)