fix(#696): git-delta scoping + file manifest for validation evidence

[lwgray]

What is this fixing?

Marcus is a multi-agent software development system where AI agents work in parallel, each coding a task and submitting it for validation. Validation checks whether the agent's code satisfies acceptance criteria by loading code files and sending them to an AI model for review.

This PR fixes a critical bug where the validator loaded the entire merged codebase (all files from all agents) instead of only the files the current agent changed. Unchecked, this caused token counts to explode, exceeding model context limits and blocking agents from completing tasks.

Why (User-Visible Problem)

In a recent run, agent `1_25` finished a task. By the time validation ran, 5 other agents had merged their work. The validator loaded all 47 files — 211,000 tokens — and hit the model's token limit. The agent was blocked. The task stayed stuck in `IN_PROGRESS` forever.

Across the last 5 projects, `validate_work` cost $0.97 — more than the planning cost for some projects. The largest single call was 122,985 input tokens to check one agent's 2-file change.

---

What Changed (Two-Part Fix)

Part 1 — Git-delta scoping (token explosion fix)

`src/core/models.py`

• Added `baseline_commit: Optional[str] = None` to `TaskAssignment`

`src/marcus_mcp/tools/task.py`

• Added `_capture_baseline_commit(assignment, project_root)` — runs `git rev-parse HEAD` at task assignment time and stores the SHA on the assignment
• Called immediately after `state.agent_tasks[agent_id] = assignment` in `request_next_task`

`src/ai/validation/work_analyzer.py`

• Added `_get_git_delta_files(project_root, state, agent_id)` — runs `git diff ..HEAD --name-only` in the agent's worktree to get only changed files; returns `None` on any failure (safe fallback to full scan)
• Updated `_discover_source_files(project_root, allowed_files=None)` — when `allowed_files` is provided, loads only those files instead of walking the whole tree

Part 2 — File manifest (false "missing file" fix)

The delta approach exposed a second problem: files like `pyproject.toml` and `Makefile` are excluded from `SOURCE_EXTENSIONS` (only `.py`, `.js` etc. are read for content). With only 3 stub `.py` files in the prompt, the LLM correctly reported "pyproject.toml is missing" — but the citation verifier requires `file:line` citations and dropped the report. Validation incorrectly passed.

Fix: collect a full file manifest (filenames only, zero content reads) from the worktree and inject it into the validation prompt:

PROJECT FILE MANIFEST (every file that exists):
  .flake8
  Makefile
  pyproject.toml
  setup.py
  src/cli_task_runner/__init__.py
  ...

NOTE: Files listed above exist. Do NOT report manifest-listed files as missing.

The LLM can now verify existence of `pyproject.toml` from the manifest even though its content is not loaded. It only flags a file as missing if it appears in neither the manifest nor the source files section.

`src/ai/validation/validation_models.py`

• Added `file_manifest: list[str]` field to `WorkEvidence`

`src/ai/validation/work_analyzer.py`

• Added `_collect_file_manifest(project_root)` — `os.walk` for names only, no file reads, excludes same dirs as `EXCLUDE_DIRS`
• Updated `gather_evidence()` to call it and populate `WorkEvidence.file_manifest`
• Updated `_build_validation_prompt()` to inject the manifest before source file content

Fallback behaviour is fully preserved: if git fails, if no baseline exists, or if `allowed_files` is `None`, the original full-worktree scan runs unchanged.

---

Token Impact

Metric	Before	After
Files with content loaded	47+ (all merged agents)	2–5 (agent's delta only)
Tokens per validation call	122,000+	~3,000–5,000
Cost per call	~$0.12	~$0.003
File existence check	Not possible	Free (manifest, names only)
Token limit failures	Yes	No

---

Test Plan

• pytest tests/unit/ai/validation/test_work_analyzer.py::TestGitDeltaEvidence — 7 tests: delta scoping, fallback on missing baseline, fallback on git failure, allowed_files allowlist, empty allowlist, missing-file tolerance
• pytest tests/unit/ai/validation/test_work_analyzer.py::TestFileManifest — 5 tests: manifest includes all extensions (.toml, Makefile, .yml), excludes EXCLUDE_DIRS, populates WorkEvidence, prompt contains manifest section, manifest appears before source file content
• pytest -m unit — 2325 passed, 0 failures, no regressions
• mypy src/ai/validation/work_analyzer.py src/ai/validation/validation_models.py src/marcus_mcp/tools/task.py src/core/models.py — clean

Related

• Closes Critical: Validation evidence collection sends entire worktree instead of agent delta — token explosion blocks parallel runs #696
• Simon blocker: df7d0aea

🤖 Generated with Claude Code

[claude]

Claude finished @lwgray's task —— View job

---

PR Review: fix(#696): scope validation evidence to agent git delta

This PR effectively addresses a critical performance issue where validation was loading entire merged codebases instead of just the files changed by individual agents. Here's my detailed review:

✅ Code Quality & Best Practices

Excellent adherence to coding standards:

• Clear, descriptive function names (_capture_baseline_commit, _get_git_delta_files)
• Comprehensive docstrings with numpy-style formatting
• Proper error handling with graceful fallbacks
• Clean separation of concerns between baseline capture and delta computation

Well-structured implementation:

• TaskAssignment.baseline_commit field properly typed as Optional[str]
• Git operations safely isolated in _capture_baseline_commit() with timeout protection
• Fallback behavior preserves existing functionality when git operations fail

✅ Potential Bugs & Issues

Robust error handling throughout:

1. Git command failures: Both _capture_baseline_commit and _get_git_delta_files gracefully handle:

   • Non-zero exit codes → logged warnings with fallback to full scan
   • Subprocess exceptions → caught and logged, no crashes
   • Timeout protection (5s for rev-parse, 10s for diff)

2. Missing baseline scenarios: When baseline_commit is None, code falls back to original full-scan behavior

3. File existence checks: Delta files that don't exist on disk are gracefully skipped

Edge cases well-covered:

• Empty git delta returns None → triggers full scan fallback
• Git repository not found → fallback behavior
• Agent has no assignment → fallback behavior

✅ Performance Considerations

Significant performance improvement:

• Before: Loading 47+ files (122,985+ tokens, $0.12+ per validation)
• After: Loading 2-5 files (~3,000-5,000 tokens, ~$0.003 per validation)
• Cost reduction: ~97% decrease in validation costs
• Scalability: O(files_changed) vs O(total_files), crucial for parallel agent workflows

Efficient implementation:

• Subprocess calls are lightweight with appropriate timeouts
• File filtering happens before content loading (memory efficient)
• Git diff with --name-only minimizes output parsing

✅ Security Considerations

Secure subprocess usage:

# Good: Fixed command arguments, no shell injection risk
subprocess.run(["git", "rev-parse", "HEAD"], ...)
subprocess.run(["git", "diff", f"{baseline}..HEAD", "--name-only"], ...)

Proper path handling:

• Uses project root resolution through workspace state
• No user-controlled path injection vectors
• File existence validation before processing

Safe fallback behavior:

• Failures never expose sensitive information
• Error messages are logged safely without exposing internals

✅ Test Coverage

Comprehensive test suite (208 lines added):

Core functionality tests:

• ✅ Git delta scoping when baseline exists (test_gather_evidence_uses_git_delta_when_baseline_commit_set)
• ✅ Fallback to full scan when no baseline (test_gather_evidence_falls_back_to_full_scan_when_no_baseline)
• ✅ Fallback when git commands fail (test_gather_evidence_falls_back_to_full_scan_when_git_fails)

Edge case coverage:

• ✅ Empty allowed files list handling
• ✅ Missing files in delta gracefully skipped
• ✅ File discovery with allowlist scoping

Integration coverage:

• ✅ End-to-end evidence gathering with agent_id parameter
• ✅ Baseline commit capture during task assignment
• ✅ Worktree resolution using explicit agent_id

Missing test considerations:

• Could add test for concurrent git operations (low risk given subprocess isolation)
• Could test very large git deltas (>100 files) but unlikely in practice

🎯 Architecture & Design

Excellent design decisions:

1. Minimal interface changes: gather_evidence() accepts optional agent_id param - backward compatible
2. Separation of concerns: Baseline capture (task assignment) vs delta computation (validation) are properly separated
3. Fail-safe design: Any failure in new git logic falls back to original behavior
4. Agent isolation: Correctly handles worktree scenarios with explicit agent_id parameter

Smart implementation details:

• Git baseline captured at assignment time, not validation time (prevents race conditions)
• Delta computation scoped to specific agent's worktree
• Allowlist approach (allowed_files) is extensible for future filtering needs

📝 Minor Suggestions

1. Consider logging delta size: Could add debug logging of actual token/byte counts saved
2. Baseline commit validation: Could add basic SHA format validation (low priority)
3. Git repository detection: Could cache git repo status to avoid repeated subprocess calls (optimization)

🎉 Summary

This is a high-quality, production-ready fix that directly addresses the core issue:

Problem: Token explosion in parallel agent validation (122K+ tokens → model failures)
Solution: Scope validation to agent's git delta only (2-5 files, ~3K tokens)
Result: 97% cost reduction, prevents context limit failures, maintains accuracy

The implementation is secure, well-tested, and maintains backward compatibility. The fallback behavior ensures system reliability even when git operations fail.

Recommendation: ✅ Approve and merge

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3794d9bd42

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve successful empty git deltas

When git diff succeeds but reports no changed files (for example, the agent has no committed source changes), this converts the empty delta to None, which gather_evidence treats as “fall back to full worktree scan.” That makes validation inspect unrelated files from the merged worktree instead of producing the existing no-source-files failure for this agent, defeating the new per-agent scoping and potentially accepting an empty completion based on other work. Return the empty list for a successful empty delta so _discover_source_files(..., allowed_files=[]) yields no evidence.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Resolve delta paths before reading

In git-delta mode the path from git diff --name-only is joined to project_path and later passed to stat()/read_text() without the resolve-and-is_relative_to guard used by the full scan. If a changed file is a source-named symlink such as leak.py pointing outside the repo, validation follows it and sends external file content to the LLM prompt. Resolve each delta path and skip anything outside project_path before reading it.

Useful? React with 👍 / 👎.

[claude]

Claude finished @lwgray's task —— View job

---

PR Review: fix(#696): scope validation evidence to agent git delta

Review Tasks:

• Read and analyze all changed files
• Evaluate code quality and best practices
• Check for potential bugs and issues
• Assess performance considerations
• Review security concerns
• Analyze test coverage
• Address codex review comments
• Provide comprehensive feedback

This PR effectively addresses a critical performance issue where validation was loading entire merged codebases instead of just the files changed by individual agents. Here's my detailed review:

✅ Code Quality & Best Practices

Excellent adherence to coding standards:

• Clear, descriptive function names (_capture_baseline_commit, _get_git_delta_files)
• Comprehensive docstrings with numpy-style formatting
• Proper error handling with graceful fallbacks
• Clean separation of concerns between baseline capture and delta computation

Well-structured implementation:

• TaskAssignment.baseline_commit field properly typed as Optional[str]
• Git operations safely isolated in _capture_baseline_commit() with timeout protection
• Fallback behavior preserves existing functionality when git operations fail

⚠️ Potential Bugs & Issues

Two significant issues identified by codex review that need attention:

1. Empty Git Delta Handling (Line 527): When git diff succeeds but reports no changed files, the code converts the empty list to None, causing fallback to full worktree scan. This defeats the scoping purpose. Should return empty list instead.

2. Symlink Security Risk (Lines 604-607): Git delta paths are not resolved/validated before reading. A malicious symlink like leak.py -> /etc/passwd could expose external files to the LLM prompt. Need path resolution and security validation.

Other edge cases well-covered:

• Git command failures → graceful fallback with logging
• Missing baseline scenarios → fallback to full scan
• File existence checks → missing files skipped gracefully
• Subprocess timeouts → 5s for rev-parse, 10s for diff

✅ Performance Considerations

Significant performance improvement:

• Before: Loading 47+ files (122,985+ tokens, $0.12+ per validation)
• After: Loading 2-5 files (~3,000-5,000 tokens, ~$0.003 per validation)
• Cost reduction: ~97% decrease in validation costs
• Scalability: O(files_changed) vs O(total_files), crucial for parallel agent workflows

Efficient implementation:

• Subprocess calls are lightweight with appropriate timeouts
• File filtering happens before content loading (memory efficient)
• Git diff with --name-only minimizes output parsing

⚠️ Security Concerns

Potential symlink vulnerability: As noted by codex, the git delta paths need resolution before reading:

# Current (vulnerable):
abs_p = project_path / rel
if abs_p.exists():
    delta_pairs.append((abs_p, abs_p))

# Should be:
abs_p = project_path / rel
resolved_path = abs_p.resolve()
if resolved_path.is_relative_to(project_path) and resolved_path.exists():
    delta_pairs.append((abs_p, resolved_path))

Secure subprocess usage:

• ✅ Fixed command arguments, no shell injection risk
• ✅ Proper working directory handling
• ✅ Safe error message logging

✅ Test Coverage

Comprehensive test suite (362 lines added):

Core functionality tests:

• ✅ Git delta scoping when baseline exists
• ✅ Fallback to full scan when no baseline
• ✅ Fallback when git commands fail
• ✅ Empty allowed files list handling
• ✅ Missing files in delta gracefully skipped
• ✅ File discovery with allowlist scoping
• ✅ End-to-end evidence gathering with agent_id parameter

Good coverage of edge cases and integration scenarios.

🎯 Architecture & Design

Excellent design decisions:

1. Minimal interface changes: gather_evidence() accepts optional agent_id param - backward compatible
2. Separation of concerns: Baseline capture (task assignment) vs delta computation (validation) properly separated
3. Fail-safe design: Any failure in new git logic falls back to original behavior
4. Agent isolation: Correctly handles worktree scenarios with explicit agent_id parameter

🔧 Recommended Fixes

Priority 1 - Security:

# In _discover_source_files around line 605:
for rel in allowed_files:
    abs_p = project_path / rel
    try:
        resolved_path = abs_p.resolve()
        if not resolved_path.is_relative_to(project_path):
            logger.warning(f"Skipping path outside project: {rel}")
            continue
        if resolved_path.exists():
            delta_pairs.append((abs_p, resolved_path))
    except (ValueError, OSError):
        logger.debug(f"Skipping invalid path: {rel}")
        continue

Priority 2 - Logic:

# In _get_git_delta_files around line 527:
return files  # Return empty list instead of None for successful empty deltas

🎉 Summary

This is a high-quality fix that directly addresses the core issue with two important security/logic fixes needed:

Problem: Token explosion in parallel agent validation (122K+ tokens → model failures)
Solution: Scope validation to agent's git delta only (2-5 files, ~3K tokens)
Result: 97% cost reduction, prevents context limit failures, maintains accuracy

The implementation is well-tested, maintains backward compatibility, and has proper fallback behavior. With the recommended fixes for the symlink security issue and empty delta handling, this will be production-ready.

Recommendation: ⚠️ Address the two identified issues, then approve and merge

---

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.