@@ -0,0 +1,57 @@
|
|||||||
|
# 🔍 Code Review Report: Commits cb88771 & f0a2103
|
||||||
|
|
||||||
|
- **Job ID**: `8bc7a4e7`
|
||||||
|
- **Reviewer**: agy (`herdr:canary-projects-multi-agent-mux-reviewer-agy`)
|
||||||
|
- **Target Commits**:
|
||||||
|
- `cb887719235277f572c40b5bf90f23efc48e612d`: `fix(resume): auto-create session entry in update_yaml_resumed.sh when missing`
|
||||||
|
- `f0a2103edf06433e7f3f9b3247df628400fe6ebb`: `refactor(isolation): simplify agent session isolation and remove legacy home-isolation helpers`
|
||||||
|
- `9ba45e5`: `test(tests): update test suite to align with isolation refactor and relative skill path`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 1. Executive Summary
|
||||||
|
|
||||||
|
This cross-code review evaluated recent architectural refactorings and bug fixes in the `multi-agent-mux` system. Specifically:
|
||||||
|
1. **Session Isolation Simplification** (`f0a2103`): Replaced complex legacy home-isolation directory copying (`.mam/agent_homes/<uuid>/`) with Universal Global Config sharing coupled with `herdr` process isolation and session-scoped conversation UUIDs.
|
||||||
|
2. **Resumed Session Auto-Creation** (`cb88771`): Added automatic session record provisioning to `update_yaml_resumed.sh` when an agent is resumed without an existing entry in `.mam/agent-sessions.yaml`.
|
||||||
|
3. **Test Suite Alignment & Verification**: Updated outdated test cases in `tests/test_tier1_unit.py`, `tests/test_tier2_component.py`, and `tests/conftest.py` to align with the simplified isolation model.
|
||||||
|
|
||||||
|
All code modifications pass syntax, runtime safety, and schema validation checks. The local commits have been pushed to `origin/main`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 2. Review Findings & Analysis
|
||||||
|
|
||||||
|
### 2.1 Commit `cb88771`: Resumed Session Auto-Creation
|
||||||
|
- **Target File**: `.agents/skills/multi-agent-mux-resume/scripts/update_yaml_resumed.sh`
|
||||||
|
- **Analysis**:
|
||||||
|
- Previously, `update_yaml_resumed.sh` failed with exit code 1 if `target is None` (session entry missing from `agent-sessions.yaml`).
|
||||||
|
- The fix gracefully instantiates a default session dictionary containing all required schema fields (`name`, `status='running'`, `role='creator'`, `pane`, `start_command`, `attach_command`, `kill_command`).
|
||||||
|
- Cleanly purges stale termination/stop metadata fields (`terminated_at`, `stopped_at`, `resumable`, etc.).
|
||||||
|
- Executes safely within `atomic_dump_yaml` transactional boundary using `BEGIN IMMEDIATE` locks.
|
||||||
|
|
||||||
|
### 2.2 Commit `f0a2103`: Session Isolation Simplification
|
||||||
|
- **Target Files**: `.agents/skills/lib.sh`, `create_session.sh`, `resume_session.sh`, `reconcile.sh`, `deploy/INSTALL.md`, `SKILL.md`
|
||||||
|
- **Analysis**:
|
||||||
|
- Removed fragile file/symlink copying routines (`provision_isolation`) that led to authentication issues and credential state desynchronization across isolated subdirectories.
|
||||||
|
- Retained backward-compatible function stubs (`provision_isolation`, `isolation_lever`, `isolation_env_prefix`, `isolation_cmd_args`) in `lib.sh` to prevent broken command invocations.
|
||||||
|
- Simplified herdr session startup commands by removing redundant isolation environment prefixes and command line arguments.
|
||||||
|
|
||||||
|
### 2.3 Test Suite Alignment
|
||||||
|
- **Target Files**: `tests/conftest.py`, `tests/test_tier1_unit.py`, `tests/test_tier2_component.py`
|
||||||
|
- **Analysis**:
|
||||||
|
- Replaced hardcoded Linux home directory paths in `conftest.py` with relative path resolution via `__file__`.
|
||||||
|
- Updated legacy test expectations for `isolation_lever`, `isolation_env_prefix`, `isolation_cmd_args`, `provision_isolation`, and `resolve_herdr_workspace`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 3. Remote Push Status
|
||||||
|
|
||||||
|
The changes have been verified and pushed to the remote repository:
|
||||||
|
```
|
||||||
|
15ffc8f..9ba45e5 main -> main
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
[VERDICT: PASS]
|
||||||
Reference in New Issue
Block a user