# Review Report: PR #38 (fix(#34): 루프가 creator의 하위 위임 잡까지 기다리고, 리뷰어에게 하위 잡 상태를 근거로 제공)

- **Job ID**: `af4c8897`
- **Reviewer**: Antigravity (`agy`, session: `herdr:reviewer-agy-01`, Role: `Reviewer`)
- **PR**: #38 (`fix/issue-34` @ `ed293dc`, base `main` `7a4a0c2` / v4.7.3)
- **Review Identity & Tree**:
  - `loop_id`: `loop-20260925T055502Z-a37cf1`
  - `base`: `7a4a0c209195c20719a50e28398416ccd19004e2`
  - `head`: `ed293dcef4c346e0050a31cb46812bcdcf1d67cf`
  - `tree`: `671b0e59eef915f89d1dce63e9c0796db132899c`
- **Verification Worktree**: `/var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20260925T055502Z-a37cf1/1`
- **Verdict**: **PASS**

---

## 1. Executive Summary & Verification Identity

PR #38 addresses Issue #34, resolving the fundamental orchestration flaw where a Creator agent delegating sub-tasks via `multi-agent-mux-delegate-job` could terminate its root job while child jobs were still running or pending. Previously, the loop immediately proceeded to the review phase, resulting in reviewers passing half-finished work or reviewing snapshots that were subsequently overwritten by asynchronous sub-jobs.

PR #38 introduces:
1. **Hierarchical Job Tracking**: Records `parent_job` in `.mam/jobs/<id>/job.json`, with strict registration-time validation, cycle detection, and limit bounding (`MAM_JOB_TREE_MAX=50`).
2. **Context Propagation & Safety Nets**: Automatic default propagation of `MAM_JOB_ID` through runner environment injection and shell wrappers, with fallback detection for unattributed open jobs created during the turn.
3. **Deterministic Job-Tree Waiting**: The orchestrator runner waits for all descendants across implementation, correction, and compliance rounds up to `MAM_CHILD_WAIT_S` (default 3900s), classifying child states into definitive categories (`OPEN`, `CLOSED-UNCONFIRMED`, `NOT-DISPATCHED`, `completed`, `error`).
4. **Structured Review Evidence & Clean Prompt Hygiene**: Reviewers receive structured markdown tables of child job states and strict review rules (`SUBJOB_RULE`). Zero sub-jobs and zero unattributed jobs produce an empty string, guaranteeing byte-for-byte prompt hygiene for un-delegated loops.
5. **Stale Review Guard**: Cryptographic fingerprinting (`_fingerprint()`) invalidates reviews if source files or worktree change during review rounds when sub-jobs exist, guarding against late writes from sub-agents closed by timeout.
6. **Cross-Workspace Herdr Scoping Fix**: Resolves pane-inherited `HERDR_WORKSPACE_ID` issues in `lib.sh` (`mam_herdr`), ensuring pane callers can locate agents in other workspaces on the same server while strictly adhering to Issue #23/#24 isolation boundaries and preflight target checks (`target_not_found`, exit 5).

### Identity & Tree Verification
The verification was executed in an isolated detached worktree at commit `ed293dc`:
- `git rev-parse HEAD` = `ed293dcef4c346e0050a31cb46812bcdcf1d67cf`
- `git rev-parse 'HEAD^{tree}'` = `671b0e59eef915f89d1dce63e9c0796db132899c`
- `git diff --check 7a4a0c2...HEAD` = Clean (0 formatting / whitespace issues)
- Worktree Status: Clean

---

## 2. Detailed Technical Audit (Items 1 – 9)

### Item 1: Parent Job Tracking & Registry Schema
- **File**: `lib_py/delegation/registry.py`
  - Added optional `parent_job: str | None = None` to `Job` dataclass and serialization schema (`to_dict` / `from_dict`).
  - `register()` validates that if `--parent-job` is supplied, the referenced parent job directory and `job.json` must exist in the registry (`jobs_dir / parent_job / "job.json"`). If nonexistent, registration halts with exit code 1 (`ValueError: parent job <id> does not exist in registry`).
  - Added query helpers:
    - `list_jobs(parent=...)`: filters jobs by parent identifier.
    - `children_of(parent_id)`: returns immediate direct child jobs.
    - `descendants_of(parent_id, max_jobs=50)`: performs a breadth-first search (BFS) traversing the entire subtree.
  - **Loop/Cycle Safety**: Maintains a `seen = {parent_id}` set. Any circular references or repeated visits are ignored. If total discovered descendants reach `max_jobs` (`MAM_JOB_TREE_MAX`, default 50), traversal halts and raises a bounded exception / warning, preventing unbounded recursion and denial-of-service.

### Item 2: Parent ID Propagation & Safety Net
- **Files**: `multi-agent-mux-delegate-job`, `create_session.sh`, `lib_py/loop/runner.py`
  - `multi-agent-mux-delegate-job submit` adds `--parent-job <id>`. If omitted, it automatically falls back to `os.environ.get("MAM_JOB_ID")`.
  - When falling back to `MAM_JOB_ID`, if the ID is not found in the registry, it logs a warning and falls back to root (allowing standalone executions outside the loop).
  - `create_session.sh` injects guidance into the agent prompt reminding creators to propagate `--parent-job "$DELEGATE_JOB_ID"` when delegating sub-tasks.
  - **Unattributed Open Jobs Safety Net**: If a sub-agent executes a delegation without passing `--parent-job`, `lib_py/loop/runner.py` detects any jobs registered during the creator's turn that remain open and unparented (`unattributed_open_jobs()`). These are surfaced in reviewer prompts under `[귀속되지 않은 열린 잡]`. If `--wait-open-jobs` is enabled, the runner automatically waits for them before review.

### Item 3: Descendant Waiting & Classification
- **File**: `lib_py/loop/runner.py`
  - Integrated `wait_for_job_tree(root_job_id)` into:
    1. Initial implementation turn (`_run_creator_round`)
    2. Correction rounds (`_run_correction_round`)
    3. Compliance audit turns (`_run_compliance_round`)
  - The runner waits for the root job, then enters `wait_for_descendants()`, polling all active descendants until all reach terminal state or the timeout (`MAM_CHILD_WAIT_S`, default 3900s) expires.
  - Sub-job classification logic (`_subjob_state()`):
    - `OPEN`: status not in `("completed", "error", "cancelled")`.
    - `CLOSED-UNCONFIRMED`: closed administratively by the orchestrator via `deadline_timeout` or `agent_blocked`, or status is `error` without a recorded error reason. Explicitly warns reviewers that the sub-agent process may still be active and writing.
    - `NOT-DISPATCHED`: cancelled prior to dispatch due to `session_busy`, `target_not_found`, or `rolled back`.
    - `completed` / `error`: standard terminal states, including error detail messages.

### Item 4: Reviewer Evidence & Prompt Hygiene
- **File**: `lib_py/loop/runner.py`
  - Generates structured markdown blocks:
    - `[하위 위임 잡]`: Markdown table containing `Job ID`, `Role`, `Target Agent`, `Status`, `Reason / Note`.
    - `[귀속되지 않은 열린 잡]`: Markdown table for unparented jobs initiated during the creator turn.
  - **Prompt Hygiene**: When `len(subjobs) == 0` and `len(unattributed) == 0`, `_subjobs_block()` returns `""`. In runs where sub-delegation is not utilized, prompt formatting is 100% byte-for-byte identical to baseline.
  - **SUBJOB_RULE**: The injected reviewer instruction mandates that reviewers must evaluate the outcome of sub-jobs and issue `[VERDICT: NOT PASS]` if any sub-job remains `OPEN`, `CLOSED-UNCONFIRMED`, or failed without a documented replacement.

### Item 5: Stale Review Guard in Non-Identity Mode
- **File**: `lib_py/loop/runner.py`
  - `_fingerprint()` computes an SHA-256 hash of `(source_head, worktree_tree(exclude=...))`.
  - Exclusions strictly ignore dynamic ephemeral paths: `.mam`, `.mam/jobs`, and delegate job log directories.
  - **Armed Condition**: The guard is armed exclusively when sub-jobs exist (`subjobs != ""`). This prevents false triggers on unpinned files or unrelated workspace churn when no child delegations took place.
  - If late writes from a timeout-closed sub-job alter the source repository during the review phase:
    - Any intermediate approvals are discarded.
    - Re-review is triggered.
    - If the change occurs on the final iteration or post-approval, the loop terminates with exit code 1 to prevent committing stale/corrupted state.

### Item 6: Same-Server Cross-Workspace Herdr Scoping
- **File**: `lib.sh` (`mam_herdr`)
  - Added helper `_herdr_scope_is_pane_inherited()`.
  - Strips `HERDR_WORKSPACE_ID` from the environment only during session lookups (`agent`, `has-session`, `kill-session`) when the workspace ID in the environment matches the caller pane's workspace prefix.
  - Preserves Issue #23/#24 cross-server boundaries and explicit caller overrides (`test_t11`).
  - Added preflight target check: if an agent target is not visible on the server, it reports `target_not_found` (exit code 5) rather than falsely classifying it as `session_busy`.

### Item 7: Test Suite & Regression Coverage
All test suites were executed inside the verification worktree:
1. `tests/test_issue34_job_tree.py`: **17/17 PASSED** (0:01:16)
   - T1: `submit` records `--parent-job` and validates parent presence.
   - T2 & T2b: Runner waits for children and grandchildren; timeout and max tree size bounded.
   - T3: Loop waits for sub-jobs before initiating review.
   - T4 & T5: Open or failed sub-jobs surface as evidence and block reviews.
   - T5b: Timeout-closed sub-job unconfirmed state and late write voids round.
   - T6: Unattributed open jobs surfaced and waited on request.
   - T7: Single job without children maintains baseline execution path.
   - T8: Stale review guard in non-worktree mode.
   - T9: Correction rounds preserve and aggregate earlier creator sub-jobs across iterations.
   - T10, T11, T12: Herdr cross-workspace lookups, caller-chosen workspace scoping, and `target_not_found` preflight handling.
2. Regression Suites:
   - `tests/test_version_consistency.py` & `tests/test_issue5_phase1_delegation.py`: **69/69 PASSED**
   - `tests/test_issue23_24_startup_path.py` & `tests/test_loop_cli.py`: **79/79 PASSED**
   - **Total Verified Tests**: **165 tests passed, 0 failures, 0 regressions**.

### Item 8 & 9: Documentation & Versioning
- `MULTI_AGENT_RULES.md`, `MULTI_AGENT_RULES.ko.md`, and `SKILL.md` have been updated to document parent job delegation requirements, reviewer evaluation rules for child jobs, and stale guard semantics.
- Version consistency is verified; v4.7.4 version bump is reserved for the orchestrator release commit in accordance with project rules.

---

## 3. Conclusion

PR #38 provides a robust, safe, and backwards-compatible implementation of hierarchical job tracking and sub-job waiting. All edge cases (cycles, timeouts, late-writing agents, cross-workspace panes, unparented delegations) are defensively handled and thoroughly covered by automated tests.

[VERDICT: PASS]