Improvement pass from llm

- tests/test_version.py: removed the brittle exact hotdog/topping/version assertion and kept the useful display contract.
  - nightshift/agents.py: normalized review next_stage / context_update values like None, null, and N/A to empty.
  - nightshift/git.py: added clearer non-git and Git safe-directory artifact messages with actionable guidance.
  - nightshift/cli.py: fixed run --all so completed dependencies remain in scope.
  - tests/test_agents.py, tests/test_git.py, tests/test_cli.py: added regression coverage.
  - docs/codex/20260521-next-improvements.md: marked the completed items.

.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.

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)

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}")

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.",
+            "",
+        ]
+    )

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)

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()

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()

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())