feat(llc): agent-to-human handoff — KB brief generation, reviewer notification (#8231)

[mrveiss]

Summary

• Extends LLCWorkItem with reviewer_user_id, reviewer_agent_id, review_brief (JSONB)
• Adds HandoffService with full agent→human handoff lifecycle (checkout validation, brief stub, lock release, Redis notification, activity log)
• Adds 4 API routes: POST /handoff/to-human, POST /review/approve, POST /review/request-changes, GET /handoff-brief
• Phase 4 brief is a structured stub from work item fields + comments; Phase 5 (feat(llc/kb): Heartbeat context builder — parallel RAG assembly, fat payload construction #8236) wires RAG

Test plan

• python -m pytest autobot-backend/llc/ -k handoff_to_human → 22 passed
• HandoffNotAllowed raised when agent does not hold checkout
• status=IN_REVIEW, reviewer_user_id set after successful handoff
• Redis checkout key deleted on handoff
• Notification published to llc:notifications:{company_id}
• Approve sets status=DONE; request-changes reverts to IN_PROGRESS with comment

Closes #8231

🤖 Generated with Claude Code

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

Code Review — PR #8511: Agent→Human Handoff

Verdict: Changes Requested — 2 blocking issues must be fixed before merge; 4 high-severity findings should also be addressed.

---

BLOCKING: Critical Issues

1. Alembic migration branch conflict — new columns never created in production

autobot-backend/migrations/versions/20260524_036_llc_work_item_review_brief.py has down_revision = "20260523_035", but 20260523_036_llc_heartbeat_scheduler.py already uses the same down_revision. This creates a branched Alembic head:

alembic upgrade head
→ FAILED: Multiple head revisions present for given argument 'head'

The migration never runs, so reviewer_user_id, reviewer_agent_id, and review_brief columns do not exist in the DB. Every call to agent_to_human, approve, or request_changes returns HTTP 500 with psycopg.errors.UndefinedColumn.

Fix: Rebase migration onto the correct down_revision (current chain tail after merging adjacent PRs).

---

2. Migration only adds review_brief — reviewer_user_id and reviewer_agent_id never created

Even if the branching issue is fixed, the migration only runs:

op.add_column("llc_work_items", sa.Column("review_brief", JSONB, nullable=True))

The model declares two additional columns (reviewer_user_id, reviewer_agent_id) with no corresponding migration. item.reviewer_user_id = uuid.UUID(reviewer_user_id) in agent_to_human raises UndefinedColumn on every flush. Also: reviewer_agent_id is never written by any code path (see finding #10).

Fix: Add both columns to the migration's upgrade().

---

HIGH: Should fix before or immediately after merge

3. lazy_singleton(HandoffService) injects no activity_log — all audit events dead in production

# work_items.py:48
_get_handoff_service = lazy_singleton(HandoffService)

HandoffService() with no arguments → self.activity_log = None. Every if self.activity_log: guard is always False. The three new ActivityEventType values (WORK_ITEM_HANDOFF, REVIEW_APPROVED, REVIEW_CHANGES_REQUESTED) are dead code in production — no audit rows ever written for handoff operations.

---

4. approve() does not clear reviewer_user_id

DONE items permanently carry a reviewer_user_id. _assert_reviewer still passes for that user on any reopened item — double-approve creates ghost audit rows. Any future query on reviewer_user_id IS NOT NULL as "active review" returns false positives on all DONE items.

Fix: Add item.reviewer_user_id = None to approve() (matching what request_changes already does at line 159).

---

5. agent_to_human() has no item.status guard

Only item.assignee_agent_id is checked — not the item's state. A DONE, CANCELLED, or already IN_REVIEW item with a matching assignee_agent_id can be re-handed-off, overwriting the reviewer and brief mid-review.

Fix: if item.status not in (WorkItemStatus.IN_PROGRESS, WorkItemStatus.READY): raise HandoffNotAllowed(...)

---

6. activity_log.record() on shared session — IntegrityError corrupts transaction silently

All three methods wrap activity logging in except Exception: logger.warning(...). If record() flushes and raises an IntegrityError, SQLAlchemy marks the session as needing rollback. The except swallows it, but the route's subsequent session.commit() raises PendingRollbackError, silently rolling back the status mutation with an unexplained 500.

Fix: Use a separate session for activity logging, or catch only ActivityLogError.

---

MEDIUM

7. redis.delete in agent_to_human unguarded after flush

A Redis error at line 116 propagates out after session.flush() — route's session.commit() never called, DB rolls back. User gets unexplained 500 on what should be a non-fatal side-effect. Wrap in try/except.

8. _publish_a2h_notification swallows at logger.debug

Reviewer never notified on Redis failure; failure invisible at default INFO log level. Sibling _publish_h2a_notification uses logger.exception. Raise to logger.warning.

9. GET /{work_item_id}/handoff-brief has no authorization

No auth dependency, no company-ID scoping. Brief contains description_excerpt, agent_notes, acceptance_criteria, comment excerpts — IDOR in multi-tenant deployment.

---

LOW

10. reviewer_agent_id declared in model but never written

Serialized in _item_to_dict, indexed in the model, but no code sets it. File a discovery issue.

---

Summary Table

#	Severity	Issue
1	BLOCKING	Alembic branch conflict — migration never runs
2	BLOCKING	Migration missing reviewer_user_id / reviewer_agent_id columns
3	HIGH	lazy_singleton no activity_log — all audit events dead in production
4	HIGH	approve() doesn't clear reviewer_user_id
5	HIGH	agent_to_human() no status guard — DONE items can be re-handed-off
6	HIGH	Activity log on shared session silently corrupts transaction
7	MEDIUM	redis.delete unguarded after flush
8	MEDIUM	_publish_a2h_notification swallows at debug level
9	MEDIUM	GET /handoff-brief no auth (IDOR)
10	LOW	reviewer_agent_id declared but never written

Core logic and 22-test coverage are solid. Fix #1 and #2 before merge; track #3–#6 with issues.