Files
multi-agent-mux/.agents/reports/reviewer-agy-01/report-99090752.md
T

7.3 KiB

Code Review Report — Job 99090752 (Issue #40: Auto-Pin of Post-Discovered Session IDs on completed)

  • Reviewer: herdr:reviewer-agy-01 (agy)
  • Iteration: 1
  • Verification Worktree: /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261020T133532Z-a1684a/1
  • Branch / Commit Range: fix/issue-40-auto-pin (ff8b381520ae04e80c73a9e18bfad73d47e853c9..feccfd5e0356c8acf0438bc68bce7ea36ce2588e, 1 commit)

1. Summary of Changes Reviewed

The implementation resolves Issue #40 by unifying post-discovered session ID capture and pinning across the completed-receiving control-plane paths and reconcile.sh drift C, while eliminating agy's cross-workspace / cross-session conversation ID contamination:

  1. Adapter Contract & Lifecycle Classification (.agents/skills/lib_py/agents/base.py, claude.py, grok.py, agy.py):
    • Introduces PRE_ASSIGNED (claude, grok) and POST_DISCOVERED (agy, opencode, codex, hermes via BaseAgentAdapter.id_lifecycle default).
    • Adds BaseAgentAdapter.capture_active_session_id(ctx) (read-only), returning capture_result(status, session_id=..., reason=..., candidates=...). For opencode, codex, and hermes, a session ID is captured only when discover(ctx) yields exactly one candidate accepted by verify_session_uuid(..., mode="discover") and not claimed in ctx.row["_sibling_claimed_uuids"].
    • AgyAgentAdapter.verify_artifact (mode="discover") now:
      • Rejects any UUID mapped by cache/last_conversations.json to another workspace (workspace_key(k) != workspace_key(ctx.cwd)).
      • Rejects any UUID in ctx.row.get("_sibling_claimed_uuids") unconditionally (eliminating the previous if not cache_match: bypass that allowed two agy sessions in the same cwd to cross-pin the same cached conversation ID).
    • AgyAgentAdapter.capture_active_session_id(ctx) treats last_conversations.json[cwd] strictly as a proposal and requires _owner_check(uuid, ctx) (lsof -t on presence/<uuid>.lock, <uuid>.db, -wal, -shm held by pane.pid or a descendant in ps -eo pid=,ppid=).
  2. Control-Plane Auto-Pin Handler (.agents/skills/lib_py/agents/autopin.py, .agents/skills/lib.sh):
    • Implements claimed_elsewhere, capture, pin_row, resume_dry_run, tui_viewport, handle_completed, and CLI python -m lib_py.agents.autopin.
    • Enforces security and trust boundaries (#15/#20/#21): only the receiving/verifying side mutates .mam/agent-sessions.yaml via atomic_dump_yaml; publish_event.py / publisher.py never touch the session registry.
    • Deduplicates per (job_id, dispatch_token, attempt_id) under fcntl.flock (<job_dir>/autopin.lock) + atomic os.replace marker (<job_dir>/autopin.json).
    • Skips remote (transport == "remote" / remote:*), human, non-completed, or stale-dispatch-token records; leaves PRE_ASSIGNED and already-pinned rows untouched; records failure/ambiguity reasons in last_visible_status without failing the completed job.
  3. Call Sites & Reconcile Unification (create_session.sh, multi-agent-mux-delegate-job, runner.py, reconcile.sh):
    • create_session.sh: invokes delegate_autopin_completed "$DELEGATE_JOB_ID" "$DELEGATE_JOB_TOKEN" "$task_reg_dir" upon onboarding completed.
    • multi-agent-mux-delegate-job: invokes delegate_autopin_completed "$JOB_ID" "$_disp_tok" "$REGISTRY_DIR" when direct job finishes completed.
    • lib_py/loop/runner.py: invokes self._autopin(job_id) in wait_for_job when status == "completed" as an idempotent recovery path (preserving G9 import isolation via subprocess invocation).
    • reconcile.sh: replaces the separate drift C blocks for agy (which previously globbed conversations/*.db across all workspaces), hermes, opencode, and codex with a single loop calling autopin.capture(s, agent, d, home_dir=home) and autopin.pin_row / autopin.resume_dry_run.

2. Verification & Test Evidence

All verification was executed against the isolated worktree /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261020T133532Z-a1684a/1 with temporary HOME and --basetemp directories (no real user/agent state databases opened):

  1. Issue #40 Completion Conditions (tests/test_issue40_auto_pin.py):
    • Condition 1 (No cross-workspace agy pinning): test_agy_verify_rejects_id_cached_for_another_workspace, test_reconcile_does_not_pin_another_workspaces_agy_conversation, test_autopin_does_not_pin_another_workspaces_agy_conversation — PASSED.
    • Condition 2 (First completed job pins without daemon): test_completed_onboarding_pins_agy_without_daemon, test_completed_first_job_pins_opencode_codex_hermes[opencode|codex|hermes], test_reconcile_pins_agy_owned_by_the_pane, test_lib_sh_helper_pins_through_the_cli, test_create_onboard_pins_without_daemon — PASSED.
    • Condition 3 (Idempotency per job/token/attempt, ambiguity & ownership handling): test_agy_same_cwd_sibling_never_cross_pins, test_agy_capture_requires_presence_owned_by_pane (4 parametrized cases), test_agy_holder_pids_reads_lsof, test_wrapper_and_loop_recovery_pin_once, test_loop_recovery_pins_when_the_wrapper_did_not, test_ambiguous_candidates_leave_reason_without_pin — PASSED.
    • Condition 4 (Pre-assigned & existing IDs untouched, ineligible/stale skipped, publisher boundary): test_preassigned_and_existing_ids_are_untouched, test_ineligible_jobs_pin_nothing (4 parametrized cases), test_stale_dispatch_token_pins_nothing, test_call_sites_and_publisher_boundary — PASSED.
  2. Regression & Component Suites:
    • pytest tests/test_issue40_auto_pin.py tests/test_issue64_ro_connect.py tests/test_issue60_linux_fixes.py tests/test_issue60_opencode_v2.py tests/test_a4_adapter_contract.py tests/test_tier1_unit.py -v → all 27 tests in tests/test_issue40_auto_pin.py and all unit/adapter/ro_connect tests PASSED (237 passed, 0 failed).
    • pytest tests/test_issue40_auto_pin.py tests/test_issue64_ro_connect.py tests/test_issue60_linux_fixes.py tests/test_issue60_opencode_v2.py tests/test_a4_adapter_contract.py tests/test_tier1_unit.py tests/test_tier2_component.py -k "opencode or codex or agy or hermes or autopin or capture or lifecycle or ro_connect or issue40" → 131 passed (0 failed).
  3. Negative Regression Verification Against Base (ff8b381520ae04e80c73a9e18bfad73d47e853c9):
    • Exported ff8b381520ae04e80c73a9e18bfad73d47e853c9 via git archive to a temporary directory and ran tests/test_issue40_auto_pin.py tests (test_adapter_id_lifecycle_classification, test_agy_verify_rejects_id_cached_for_another_workspace, test_reconcile_does_not_pin_another_workspaces_agy_conversation, test_call_sites_and_publisher_boundary) → 4 failed (BASE_RC=1), confirming the tests directly catch the pre-fix behavior (including reconcile.sh pinning proj-B's UB onto proj-A's row).
  4. Static & Hygiene Checks:
    • git diff --check ff8b381520ae04e80c73a9e18bfad73d47e853c9..feccfd5e0356c8acf0438bc68bce7ea36ce2588e → clean (exit 0).
    • bash -n on .agents/skills/lib.sh, create_session.sh, multi-agent-mux-delegate-job, and reconcile.sh → clean (exit 0).
    • git status --short in verification worktree → clean.

3. Findings

No blocking issues, regressions, or defects found.

[VERDICT: PASS]