diff --git a/.gitignore b/.gitignore index 450e5b2..5fe1f55 100644 --- a/.gitignore +++ b/.gitignore @@ -25,7 +25,7 @@ share/python-wheels/ .installed.cfg *.egg MANIFEST - +docs/codex/ # PyInstaller # Usually these files are written by a python script from a template # before PyInstaller builds the exe, so as to inject date/other infos into it. diff --git a/nightshift/agents.py b/nightshift/agents.py index a889240..d27031c 100644 --- a/nightshift/agents.py +++ b/nightshift/agents.py @@ -511,11 +511,20 @@ def parse_review_output(output: str) -> tuple[StageStatus, str, str | None, str return "fail", "Review output did not include a valid status.", None, None reason = values.get("reason") or "Review returned no reason." - next_stage = values.get("next_stage") or None - context_update = values.get("context_update") or None + next_stage = _optional_review_value(values.get("next_stage")) + context_update = _optional_review_value(values.get("context_update")) return raw_status, reason, next_stage, context_update # type: ignore[return-value] +def _optional_review_value(value: str | None) -> str | None: + if value is None: + return None + normalized = value.strip() + if not normalized or normalized.lower() in {"none", "null", "n/a"}: + return None + return normalized + + def format_agent_invocation(stage_id: str, invocation: AgentInvocation) -> str: stdout = _coerce_output(invocation.stdout) stderr = _coerce_output(invocation.stderr) diff --git a/nightshift/cli.py b/nightshift/cli.py index 9bd364a..2114661 100644 --- a/nightshift/cli.py +++ b/nightshift/cli.py @@ -176,13 +176,12 @@ def main(argv: list[str] | None = None) -> int: parser.error("run accepts either --all or --task, not both.") runner = PipelineRunner(config, logger=RunLogger(console=print)) if args.all: - selected = [task for task in tasks if not task.completed] with TerminalAnimation( args.animation, message="NightShift running all tasks", enabled=not args.no_animation, ): - result = runner.run_tasks(selected) + result = runner.run_tasks(tasks) print(f"Status: {result.status}") print(f"Tasks run: {len(result.task_results)}") print(f"Completed: {result.completed_count}") diff --git a/nightshift/git.py b/nightshift/git.py index 5525598..4a42de1 100644 --- a/nightshift/git.py +++ b/nightshift/git.py @@ -52,6 +52,29 @@ def is_git_repository(project_root: Path) -> bool: return state.available and state.stdout.strip() == "true" +def git_failure_reason(result: GitCommandResult, project_root: Path) -> str: + details = (result.stderr or result.stdout or "unknown git error").strip() + lowered = details.lower() + if "dubious ownership" in lowered or "safe.directory" in lowered: + return "\n".join( + [ + "Git repository ownership is not trusted by Git.", + "", + "Git refused to read this repository because it appears to be owned by a different user identity.", + "NightShift will not change global Git configuration automatically.", + "", + "To trust this repository, run:", + "", + "```powershell", + f"git config --global --add safe.directory {project_root.as_posix()}", + "```", + ] + ) + if "not a git repository" in lowered or "not a git work tree" in lowered: + return "Project root is not a git repository." + return details or "unknown git error" + + def ensure_clean_worktree(project_root: Path, require_clean: bool) -> None: if not require_clean: return @@ -59,27 +82,31 @@ def ensure_clean_worktree(project_root: Path, require_clean: bool) -> None: if not status.available: raise SafetyError( "Safety error: clean worktree is required, but git status could not be read: " - f"{status.stderr.strip() or 'unknown git error'}" + f"{git_failure_reason(status, project_root)}" ) if status.stdout.strip(): raise SafetyError("Safety error: clean worktree is required, but repository is dirty.") def write_git_artifacts(artifacts: ArtifactStore, task_id: str, when: str) -> Path: + state = get_git_repository_state(artifacts.project_root) + if not state.available: + content = format_git_unavailable_status(state, when, artifacts.project_root) + return artifacts.write_stage_output(task_id, f"git-status-{when}.txt", content) status = get_git_status(artifacts.project_root) content = format_git_status(status, when) return artifacts.write_stage_output(task_id, f"git-status-{when}.txt", content) 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" + state = get_git_repository_state(artifacts.project_root) + if not state.available or state.stdout.strip() != "true": + content = f"Git diff unavailable.\n\nReason: {git_failure_reason(state, artifacts.project_root)}\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: - details = (diff.stderr or "unknown git error").strip() - content = f"Git diff unavailable.\n\nReason: {details}\n" + content = f"Git diff unavailable.\n\nReason: {git_failure_reason(diff, artifacts.project_root)}\n" elif diff.stdout: content = diff.stdout else: @@ -108,3 +135,22 @@ def format_git_status(status: GitCommandResult, when: str) -> str: "", ] return "\n".join(lines) + + +def format_git_unavailable_status(status: GitCommandResult, when: str, project_root: Path) -> str: + return "\n".join( + [ + f"# Git Status {when}", + "", + "Git repository: false", + f"Available: {str(status.available).lower()}", + f"Exit code: {status.exit_code}", + "", + "## Explanation", + "", + git_failure_reason(status, project_root), + "", + "Git metadata and diff artifacts are unavailable for this run.", + "", + ] + ) diff --git a/tests/test_agents.py b/tests/test_agents.py index 03170ed..016aae9 100644 --- a/tests/test_agents.py +++ b/tests/test_agents.py @@ -110,6 +110,18 @@ class AgentExecutorTests(unittest.TestCase): self.assertEqual(next_stage, "implement") self.assertEqual(context_update, "Fix tests") + def test_review_output_parser_treats_empty_sentinel_next_stage_as_missing(self) -> None: + for next_stage_value in ("", "None", "null", "N/A"): + with self.subTest(next_stage=next_stage_value): + status, reason, next_stage, context_update = parse_review_output( + f"status: pass\nreason: ok\nnext_stage: {next_stage_value}\ncontext_update: None\n" + ) + + self.assertEqual(status, "pass") + self.assertEqual(reason, "ok") + self.assertIsNone(next_stage) + self.assertIsNone(context_update) + def test_ollama_agent_invocation_uses_model_without_real_ollama(self) -> None: with tempfile.TemporaryDirectory() as directory: root = Path(directory) diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..f5fd829 --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,92 @@ +from pathlib import Path +import subprocess +import sys +import tempfile +import unittest + + +class CliTests(unittest.TestCase): + def test_run_all_keeps_completed_dependencies_in_scope(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + (root / "dummy.md").write_text("Unused.", encoding="utf-8") + (root / "nightshift.yaml").write_text( + "\n".join( + [ + "project:", + " name: cli-test", + " root: .", + " task_file: tasks.md", + " artifact_dir: .nightshift", + "", + "safety:", + " require_clean_worktree: false", + " scoped_paths:", + " - .", + " allowed_commands:", + " - python -c \"print('ok')\"", + " forbidden_commands:", + " - rm -rf", + "", + "agents:", + " dummy:", + " backend: command", + " command: python -c \"print('unused')\"", + " system_prompt: dummy.md", + "", + "pipeline:", + " max_task_retries: 0", + " stages:", + " - id: test", + " type: command", + " commands:", + " - python -c \"print('ok')\"", + " output: test-output.txt", + ] + ), + encoding="utf-8", + ) + (root / "tasks.md").write_text( + """# Tasks + +- [x] TASK-001: Already complete + +Acceptance Criteria: +- done + +- [ ] TASK-002: Depends on completed task + +Dependencies: +- TASK-001 + +Acceptance Criteria: +- runs +""", + encoding="utf-8", + ) + + completed = subprocess.run( + [ + sys.executable, + "-m", + "nightshift.cli", + "run", + "--config", + str(root / "nightshift.yaml"), + "--all", + "--no-animation", + ], + cwd=root, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + + self.assertEqual(completed.returncode, 0, completed.stdout + completed.stderr) + self.assertIn("Completed: 1", completed.stdout) + self.assertIn("- [x] TASK-002", (root / "tasks.md").read_text(encoding="utf-8")) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_git.py b/tests/test_git.py index 6df51b8..de7bc90 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -6,7 +6,14 @@ import unittest from nightshift.artifacts import ArtifactStore from nightshift.errors import SafetyError -from nightshift.git import ensure_clean_worktree, write_diff_artifact, write_git_artifacts +from nightshift.git import ( + GitCommandResult, + ensure_clean_worktree, + format_git_unavailable_status, + git_failure_reason, + write_diff_artifact, + write_git_artifacts, +) def git_available() -> bool: @@ -46,9 +53,41 @@ class GitSafetyTests(unittest.TestCase): 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.assertIn("Project root is not a git repository", content) self.assertNotIn("usage: git diff", content) + def test_git_status_artifact_is_readable_outside_git_repo(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + artifacts = ArtifactStore(root, ".nightshift", run_id="test-run") + + status_path = write_git_artifacts(artifacts, "TASK-001", "before") + + content = status_path.read_text(encoding="utf-8") + self.assertIn("Git repository: false", content) + self.assertIn("Project root is not a git repository", content) + self.assertNotIn("fatal:", content) + + def test_safe_directory_failure_gets_actionable_guidance(self) -> None: + root = Path("C:/repo/project") + result = GitCommandResult( + available=False, + exit_code=128, + stdout="", + stderr=( + "fatal: detected dubious ownership in repository at 'C:/repo/project'\n" + "To add an exception for this directory, call:\n" + "git config --global --add safe.directory C:/repo/project\n" + ), + ) + + reason = git_failure_reason(result, root) + formatted = format_git_unavailable_status(result, "before", root) + + self.assertIn("ownership is not trusted", reason) + self.assertIn("git config --global --add safe.directory C:/repo/project", reason) + self.assertIn("NightShift will not change global Git configuration", formatted) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_version.py b/tests/test_version.py index a0c0088..e04c2a4 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -15,10 +15,11 @@ from nightshift.version import ( class VersionTests(unittest.TestCase): def test_display_version_includes_channel_hotdog_and_topping(self) -> None: - self.assertEqual(display_version(), "0.2.4-alpha-bratwurst-relish") - self.assertEqual(PACKAGE_VERSION, "0.2.4") + self.assertTrue(display_version().startswith(f"{PACKAGE_VERSION}-alpha-")) self.assertIn(hotdog_version, HOTDOG_VERSIONS) self.assertIn(topping_version, TOPPING_VERSIONS) + self.assertIn(hotdog_version, display_version()) + self.assertIn(topping_version, display_version()) def test_banner_uses_central_display_version(self) -> None: self.assertIn(f"VERSION: {display_version()}", format_banner())