Files
multi-agent-mux/.agents/reports/reviewer-agy-01/report-4be97273.md
T
GodopuandClaude Opus 5.5 121eab71d4 release(v4.9.3): u1e no longer writes the checkout's .mam/shim/herdr - archive the review reports and bump the lockstep version
Review loop (--review-only, --all-reviewer): round 1 (@3a30d7d) 4/4 PASS;
each reviewer reproduced W1 failing on the pre-fix tree. Seven #50/#55 and
leak modules (112 tests) from a clean worktree left no .mam in the
checkout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
2026-10-13 12:33:02 +09:00

6.9 KiB

Code Review Report: fix/u1e-workspace-root (Test Isolation Fix for test_u1e)

  • Job ID: 4be97273
  • Reviewer: agy (Session: herdr:reviewer-agy-01, Role: Reviewer)
  • Date: 2026-10-13
  • Target Branch: fix/u1e-workspace-root
  • Base Commit: 42e2a7ea9bd3e2770ed8626d789193428ca3aa8b
  • Head Commit: 3a30d7da894cdac26b7ec054b4e66fe9fa3dc8db
  • Verification Path: /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261013T025803Z-c69408/1
  • Verification Environment: macOS (Darwin 24.3.0, arm64), Python 3.9.6, pytest-8.4.2

1. Executive Summary

This review evaluated commit 3a30d7da894cdac26b7ec054b4e66fe9fa3dc8db, which fixes a test workspace isolation leak discovered during earlier review runs in tests/test_issue50_dispatch.py::test_u1e_a_dry_run_record_is_not_an_open_job and adds regression coverage to tests/test_issue17_workspace_root_isolation.py.

The root cause was confirmed, the fix was verified both positively and negatively (reproducing failure on base, success on head), and an exhaustive audit across tests/ confirmed no other tests suffer from this defect.

Verdict: PASS.


2. Root Cause Analysis

In tests/test_issue50_dispatch.py, test_u1e_a_dry_run_record_is_not_an_open_job invokes subprocess.run on WRAPPER (multi-agent-mux-delegate-job).

  1. WRAPPER sources .agents/skills/lib.sh.
  2. In lib.sh:26, if WORKSPACE_ROOT is unset in the execution environment, it defaults to:
    WORKSPACE_ROOT="${WORKSPACE_ROOT:-$(cd "$SKILL_DIR/../.." && pwd)}"
    
    which points to the repository checkout root.
  3. In lib.sh:149-155 and lib.sh:3168, _init_herdr_isolation automatically creates and populates $WORKSPACE_ROOT/.mam/shim/herdr.
  4. Because u1e previously created a custom env containing HOME and MAM_ENV_FILE but omitted WORKSPACE_ROOT, executing u1e wrote directly into <checkout>/.mam/shim/herdr.

3. Verification of Fix & Regression Coverage

3.1 The Fix

In tests/test_issue50_dispatch.py:169-170, WORKSPACE_ROOT=str(tmp_path) was added to env:

env = dict(os.environ, HOME=str(tmp_path), MAM_ENV_FILE=str(tmp_path / ".mam.env.absent"),
           WORKSPACE_ROOT=str(tmp_path))

This guarantees that any .mam/shim creation is strictly contained within tmp_path.

In tests/test_issue17_workspace_root_isolation.py:43, "tests/test_issue50_dispatch.py::test_u1e_a_dry_run_record_is_not_an_open_job" was appended to NODES. Test W1 copies the checkout into a temporary sandbox and executes every node in NODES, asserting that no .mam directory is created in the copy.

3.2 Negative Reproduction (Pre-fix Validation)

In an isolated temporary clone checked out at base commit 42e2a7ea9bd3e2770ed8626d789193428ca3aa8b, u1e was added to NODES before the fix. Running W1 failed immediately with:

AssertionError: tests wrote into the checkout they ran in: ['.mam/shim', '.mam/shim/herdr']
assert ['.mam/shim',...m/shim/herdr'] == []
FAILED tests/test_issue17_workspace_root_isolation.py::test_w1_no_test_writes_into_the_checkout_it_runs_in

This confirms that W1 effectively catches the defect.

3.3 Positive Execution (Post-fix Validation)

Ran tests/test_issue17_workspace_root_isolation.py in the verification directory with sanitized environment and isolated --basetemp:

tests/test_issue17_workspace_root_isolation.py ..                        [100%]
============================== 2 passed in 30.47s ==============================

Verified that no .mam was created in the verification tree (ls: .mam: No such file or directory, git status clean).


4. Codebase Audit for Similar Invocations

An automated search across all test modules in tests/ identified all other locations where WRAPPER or lib.sh are invoked:

No other un-isolated invocations exist in the test suite.


[VERDICT: PASS]