GodopuandClaude Fable 5.1
22586d2a01
Issue #34: the loop waits for the creator job's sub-jobs, reviewers get the sub-job table as evidence, a run with sub-jobs voids a round whose source changed during review, and a pane caller reaches an agent of another workspace on its own server. VERSIONS.md carries the entry; the merge-side reports (round 1 @ed293dc and round 2 @e80186f with identity sidecars, baseed293dc-> heade80186f, evidence review-tree) are archived under .agents/reports/<session>/. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
123 lines
10 KiB
Markdown
123 lines
10 KiB
Markdown
# 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]
|