fix: harden reviewer registration and scoped resume context

[justinmoon]

Summary

• prevent ghost reviewer registration in implement sessions (slot bounds, no bare review fallback, capacity guard, role validation)
• scope saved run context by (workspace, session, agent) for exact-match resume lookup
• keep workspace-level context only for session/workflow continuity, not cross-agent role reuse
• add tests for reviewer-slot enforcement and identity-scoped saved-context behavior

Validation

• just pre-merge
• targeted tests in command_surface for identity lookup and multi-agent saved-resume gating

Summary by CodeRabbit

Release Notes

• New Features

  • Context is now automatically saved per workspace and identity, enabling better persistence across sessions.
  • Implement sessions now automatically assign reviewer roles when joining.

• Improvements

  • Enhanced agent selection logic with improved fallback handling and resolution pathways.
  • Added validation for reviewer slot availability and implement session capacity constraints.

• Tests

  • Added test coverage for identity-based context lookups and reviewer slot management.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR adds identity-aware saved run context storage with per-workspace and per-identity lookups, introduces agent resolution pathways that prefer saved identities, and implements sophisticated reviewer slot auto-assignment logic for Implement sessions with validation and fallback mechanisms.

Changes

Cohort / File(s)	Summary
Identity-aware context and agent resolution src/command_surface.rs	Introduces per-workspace and per-identity SavedRunContext storage. Adds resolve_run_command_requested_agent and resolve_saved_agent_fallback for identity-based agent selection. Extends ensure_agent_registered with intricate reviewer slot auto-assignment logic: single slot auto-selection, slot exhaustion handling, and identity-based overrides. Adds context_identity_key and find_saved_context_by_identity helpers and remember_saved_context to persist context after runs.
Session creation and registration enhancements src/commands.rs	Expands create_with_registry to support Implement and Workflow session types with Git repository validation and workflow state initialization. Enhances join_with_registry with reviewer slot assignment logic that auto-selects review-N when conditions allow, enforces slot constraints, and validates agent assignments. Adds internal validation helpers: reviewer_slot_count, parse_reviewer_index, is_valid_reviewer_name, validate_new_implement_agent, and implement_registration_complete.
Test coverage updates tests/command_install_run.rs, tests/extensibility_mvp.rs	Updates command_install_run test to run on "implement" step instead of "review" and assert new agent key presence. Adds implement_join_enforces_reviewer_slots_and_valid_agent_names test validating reviewer slot enforcement, invalid agent rejection, and session state consistency.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Auto-number reviewers, verb-form roles, and rally reset #18: Modifies agent registration and reviewer auto-assignment paths directly related to the reviewer-N slot handling logic in join_with_registry.
• Refactor wrapper surface to skill and externalize prompts #15: Changes command_surface.rs and alters run-context/agent resolution pathways that overlap with the identity-aware resolution logic introduced here.

Poem

🐰 A context saved by identity true,
Reviewer slots assigned—just one or two,
When agents return with a familiar face,
They find their spot in the workspace place.
No more confusion, the workflow's blessed! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.03% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: harden reviewer registration and scoped resume context' directly aligns with the main changes: preventing ghost reviewer registration through slot enforcement and scoping saved run context by identity for proper resume behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/ghost-agent-guardrails-20260305

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[devin-ai-integration]

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

[devin-ai-integration]

🔴 Deadlock: ensure_agent_registered holds exclusive session lock while calling join_with_registry which re-acquires it

When agent == "review", ensure_agent_registered opens a SessionHandle at src/command_surface.rs:1233 which acquires an exclusive file lock via fs2::lock_exclusive() (flock(LOCK_EX) on Linux). The handle variable remains in scope (holding the lock) when commands::join_with_registry is called at lines 1241 and 1282. join_with_registry (src/commands.rs:190) opens its own SessionHandle on the same session, attempting to acquire the same exclusive lock on a different file description. On Linux, flock locks are per-open-file-description, so the second lock_exclusive() blocks forever waiting for the first lock — held by the same thread — causing a deadlock.

This affects two code paths:

• Line 1241: non-Implement sessions where agent is "review" and not already registered
• Line 1282: Implement sessions where reviewer slots still have room

The old code did not have this problem because the "review" path went directly to join_with_registry without first opening a SessionHandle.

(Refers to lines 1233-1289)

Prompt for agents

In src/command_surface.rs, the ensure_agent_registered function (starting around line 1203) must drop the SessionHandle before calling commands::join_with_registry. The handle is opened at line 1233 and holds an exclusive flock. It must be dropped before the join_with_registry calls at lines 1241 and 1282.

One approach: extract the needed data (session_type, agents map, expected_agents) from the handle/state into local variables, then explicitly drop the handle before any branch that calls join_with_registry. For example:

1. After line 1235 (let state = handle.load_state()?;), copy the fields you need into locals:
   let session_type = state.session_type.clone();
   let agents = state.agents.clone();
   let expected_agents = state.config.expected_agents;
2. Then drop(handle); drop(state);
3. Use the local copies for the remaining logic.

This ensures the exclusive lock is released before join_with_registry tries to re-acquire it.

---

Was this helpful? React with 👍 or 👎 to provide feedback.