fix(conductor): stop bridge.py from spawning duplicate conductor sessions

[youling66]

Fixes #1608.

Original implementation and authorship by @youling66. Maintainer integration commits add migration safety and regression coverage without squashing the contributor commits.

Root cause

The bridge identified conductor sessions only by the expected conductor-<name> title. Existing sessions whose title had already drifted were invisible to that lookup, so an upgrade could still create another row. Separately, an uncaught Telegram, Slack, or Discord polling exception escaped asyncio.gather() and caused the service manager to restart the bridge repeatedly.

Fix

• Keep --title-lock on every newly created conductor session.
• Preserve the healthy exact-title fast path from Restarting the conductor bridge registers a duplicate conductor instance (phantom) instead of reusing the existing one #1351.
• Fall back to the profile-scoped list --json record and canonical conductor project path for drifted sessions.
• Require a unique canonical-path match, then restore the title through the stable session ID. The title mutation also locks future synchronization.
• Fail closed when list output is unavailable, malformed, has an invalid schema, the canonical path is ambiguous, or canonical-path and exact-title matches identify different sessions.
• Wrap Telegram, Slack, and Discord polling with one sequential retry loop per platform. CancelledError propagates, normal return ends the wrapper, failures log loudly, and backoff grows from 5 seconds to a 300 second cap.

Surfaces

• Conductor bridge lifecycle: covered by conductor/tests/test_conductor_title_lock.py, conductor/tests/test_bridge_task_resilience.py, and the updated Restarting the conductor bridge registers a duplicate conductor instance (phantom) instead of reusing the existing one #1351 compatibility tests.
• CLI: no new command or output shape. The bridge consumes existing profile-scoped id, title, and path fields from list --json and existing session set behavior.
• Persistence: no schema change.
• TUI, Web UI, remote sessions, and cross-platform Go behavior: no user-interface or session-model change.

Verification

• Full conductor Python suite: 72 passed, 8 dependency-gated skips.
• Bridge compile and import: Python 3.9, 3.12, and 3.13 passed locally. GitHub compatibility checks passed on Python 3.8 through 3.12.
• Relevant conductor Go tests: passed under isolated HOME and XDG directories.
• go build ./...: passed.
• golangci-lint v2.12.2: passed locally and in GitHub Actions.
• govulncheck ./...: zero reachable vulnerabilities locally and passed in GitHub Actions.
• Full race suite: all packages passed except two failures that reproduce identically on untouched current main: TestForbidigo_BansRawOSEnvironInSpawnPath and TestGetJSONLPathChecked_SameIDDifferentProjectIsNotCollision.
• git diff --check: passed.
• CodeQL, diff-scope, and CodeRabbit checks: passed.

Do not merge as part of this integration task.

[coderabbitai]

📝 Walkthrough

Walkthrough

The conductor bridge now uses fail-closed, path-based session deduplication with title locking and supervises Telegram, Slack, and Discord tasks with cancellation-aware exponential retry backoff. Regression tests cover session identity, retries, cancellation, overlap, and capped delays.

Changes

Conductor bridge resilience

Layer / File(s)	Summary
Harden conductor session identity internal/session/conductor_bridge.py, conductor/tests/test_conductor_title_lock.py, conductor/tests/test_issue1351_conductor_dedupe.py	Session listing can fail closed, conductor sessions are matched by canonical path, ambiguity prevents creation, title drift is repaired, and new sessions use --title-lock.
Retry platform integration tasks internal/session/conductor_bridge.py, conductor/tests/test_bridge_task_resilience.py	Telegram, Slack, and Discord tasks run through _run_platform_task, which retries failures with capped exponential backoff while propagating cancellation. Tests verify retry, success, cancellation, non-overlap, logging, and delay behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• asheshgoplani/agent-deck#1380: Updates related conductor session deduplication logic and regression tests.

Suggested reviewers: asheshgoplani

Sequence Diagram(s)

sequenceDiagram
  participant main
  participant asyncio.gather
  participant _run_platform_task
  participant PlatformIntegration
  main->>asyncio.gather: await supervised platform tasks
  asyncio.gather->>_run_platform_task: start task supervisor
  _run_platform_task->>PlatformIntegration: execute integration
  PlatformIntegration-->>_run_platform_task: failure or success
  _run_platform_task->>_run_platform_task: calculate capped backoff
  _run_platform_task->>PlatformIntegration: retry after sleep

Loading

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (6 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes implement the requested title-lock and retry-with-backoff fixes for #1608 and add regression coverage.
Out of Scope Changes check	✅ Passed	The changes stay focused on conductor-session deduplication and task retry handling, with no obvious unrelated additions.
Remote_parity	✅ Passed	PR only changes internal/session and tests; no files under internal/ui/ or cmd/agent-deck/ were touched, so the RemoteSession parity check is not applicable.
Test_coverage_per_surface	✅ Passed	Bridge-only changes are covered: conductor tests assert title-lock/dedupe/fail-closed paths, and task-resilience tests cover retry/backoff/cancellation; no TUI/Web surface is introduced.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title uses Conventional Commits format and clearly describes the duplicate conductor session fix.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

internal/session/conductor_bridge.py (1)

2887-2887: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a type hint for coro_factory.

The coro_factory parameter lacks a type annotation. Adding Callable[[], Awaitable[None]] (or Callable[[], Coroutine[Any, Any, None]]) would improve readability and IDE support.

♻️ Optional type hint

-async def _run_platform_task(name: str, coro_factory, max_backoff: int = 300) -> None:
+from typing import Awaitable, Callable
+
+async def _run_platform_task(
+    name: str, coro_factory: Callable[[], Awaitable[None]], max_backoff: int = 300
+) -> None:

As per path instructions, internal/session/** requires extra attention to signal handling — the explicit except asyncio.CancelledError: raise before the broad Exception catch correctly ensures SIGINT/SIGTERM cancellation propagates for clean teardown.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/session/conductor_bridge.py` at line 2887, Update the
_run_platform_task function signature by annotating coro_factory as a
zero-argument callable returning an awaitable of None, using the appropriate
typing imports. Preserve the existing cancellation handling, including explicit
propagation of asyncio.CancelledError before the broad Exception catch.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/session/conductor_bridge.py`:
- Line 2887: Update the _run_platform_task function signature by annotating
coro_factory as a zero-argument callable returning an awaitable of None, using
the appropriate typing imports. Preserve the existing cancellation handling,
including explicit propagation of asyncio.CancelledError before the broad
Exception catch.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: de0ee0a6-2a97-463f-87f8-cd2c4a8ee9d4

📥 Commits

Reviewing files that changed from the base of the PR and between 350a640 and 35d6a36.

📒 Files selected for processing (3)

• conductor/tests/test_bridge_task_resilience.py
• conductor/tests/test_conductor_title_lock.py
• internal/session/conductor_bridge.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/session/conductor_bridge.py`:
- Around line 976-1014: Update the session selection logic around
_find_session_by_title and the path_match or exact_match assignment to detect
when path_match and exact_match both exist but refer to different sessions. Fail
migration closed with an error before invoking the rename through run_cli, while
preserving the existing behavior when only one match exists or both identify the
same session.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 30c2b993-f2ff-4c48-b55a-c09e8f3c05b9

📥 Commits

Reviewing files that changed from the base of the PR and between 33ec0f9 and 7dc167e.

📒 Files selected for processing (4)

• conductor/tests/test_bridge_task_resilience.py
• conductor/tests/test_conductor_title_lock.py
• conductor/tests/test_issue1351_conductor_dedupe.py
• internal/session/conductor_bridge.py

[youling66]

Pulled these commits and ran them on the machine that originally hit #1608 — full test suite passes (73/73, aside from 6 pre-existing venv-only failures unrelated to this change), and the deployed process has now run 19+ hours straight with zero restarts and the conductor session count holding steady at 1, including surviving a real Telegram "Bad Gateway" blip along the way without crashing. Looks solid to me.