AK-134 - Conversation Initiation from Agent

[malithaYL]

Description

The design for initiation of a conversation from agent, implementation and an api example

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Test update
• CI/CD update
• Other (please describe):

Related Issues

Fixes #
Relates to #

Changes Made

Testing

• Unit tests pass locally
• Integration tests pass locally
• Manual testing completed
• New tests added for changes

Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

[amithad]

Spec-only PR (design.md / spec.md / plan.md for AK-134) — reviewed against ak-dev-write-spec, ak-dev-architecture, and the messaging-integration conventions. No implementation code, so spec-conformance checks on code don't apply.

Overall: a strong, well-verified spec set.

• Nearly every path:line claim checks out against develop: the integration session-id derivation points (Slack/WhatsApp/Telegram/Messenger/Instagram/Teams/Gmail), queue pipeline citations, response-handler behavior, thread config gate, ConversationThreadManager signatures (first_prompt, append_message, RLock), SQSHandler.send_message_to_output_queue kwargs, SessionStoreBuilder's valkey ImportError pattern, the Terraform response-store pattern, and the negative grep-claims (resolve_session_id unused in ak-py/src; no serverless DynamoDB tables).
• Design → spec → plan traceability is complete: every design requirement maps to a spec section, every spec component to exactly one plan iteration, and the tests + docs/skills-sync iterations are present.
• The error-handling table covers the details specs routinely omit: exception scopes, the singleton concurrency contract, the hot-path lookup cost, and the duplicate-send-on-redelivery rationale for complete() never raising.

Findings (inline): 1 suggestion, 2 questions, 3 nits.

• [suggestion] spec.md:103 — reuse the shared core/util/driver/ drivers instead of writing new per-module thin drivers; the RedisDriver citation points at the wrong file.
• [question] spec.md:95 — the reverse session→thread lookup has no named consumer.
• [question] plan.md:82 — the flat specs slated for deletion don't exist on develop.
• [nit]s: thread_id naming collision in the manager signatures; "fixed or agent-generated" leftover; PNG vs Mermaid diagram.

Not anchorable inline:

• [question] Staging: all three documents land in one PR, while ak-dev-write-spec expects design.md to survive its review cycles before spec.md is written. The docs hint a v1 cycle happened (plan.md's "superseded flat specs", spec.md's "v1 same-session guard") — please state in the PR description where the design review took place.
• [nit] PR title/template: spec-only PRs take a docs: conventional title (e.g. docs: add design, spec and plan for AK-134 agent-initiated conversations), the template should mark Documentation update rather than New feature, note that implementation follows in a separate PR, and mark the N/A testing/checklist items as such.
• Trivial citation drift: the thread: root field is at config.py:393, not :397 (cited in both design.md and spec.md).
• CI: the two failing e2e jobs (api/multimodal/openai, memory/key-value-cache) can't be caused by a docs-only diff — likely flaky/infra; worth a re-run before merge.

[amithad]

Re-review — the PR grew from spec-only into the full implementation (core initiation/ package, deployment wiring, 8 integration resolve points, Terraform, docs, skills, example, 4 test files). Reviewed the revised spec set first, then walked the implementation against the spec's requirements checklist.

Previous review: all six findings addressed — Mermaid diagram, shared-driver reuse (with corrected citations), the reverse-lookup consumer named in design+spec and implemented (get_messaging_integration_thread_id + reply-delivery contract), naming fixes, flat-spec plan step dropped, config.py:393. The spec revisions are consistent with the implementation.

Spec conformance: excellent. Every design/spec requirement I checked is implemented and tested: presence-gated feature (mapping_table), platform-blind runner (tool dispatches; Terraform grants the runner no table access), single choke points (resolve_session_id / complete()), never-raising complete() with the duplicate-send rationale, INITIATION guards in all four stock handler methods, save-if-absent bind, all eight integration derivation points wrapped, POST /api/v1/chat rewrite-and-return with tests, valkey ImportError pattern, target_details kept off the tool schema (strict-schema constraint, documented in spec). Tests cover both SQS record shapes, the override contract end-to-end (send → bind → AK thread), and the never-raise guarantees. The skills/docs sync is unusually thorough.

Findings (1 blocker, 1 suggestion, 1 nit — inline):

• [blocker] examples/api/slack-initiation/server.py:91 — the example's flagship DM flow never resolves replies: it binds the DM channel id, but the Slack handler resolves thread_ts, so Monroe's reply always lands in a fresh, context-less session — the exact failure AK-134 exists to fix.
• [suggestion] docs/docs/advanced/conversation-initiation.md:2 — the new page is missing from docs/sidebars.js (explicit items list), so it's unreachable from the docs navigation.
• [nit] docs/specs/ak-134/spec.md:206 — spec's Terraform/IAM wording no longer matches the (better) implementation; align it since the spec ships in this PR.

Not anchorable inline:

• [nit] plan.md iteration 10.2 commits to updating docs/docs/deployment/ pages that describe response-handler customization — none were touched. deployment/aws-containerized.md:143 ("each writing … records to the response store … On permanent failure … writes an error record") and advanced/queue-mode-guide.md's ECSOutputConsumer descriptions now have an INITIATION exception; a one-line note + link to the new page would close the gap (or record in plan.md that no update was needed).
• [nit] PR title: now an implementation PR — conventional-commit form per the code-quality guidelines, e.g. feat: agent-initiated conversations via messaging platforms (AK-134); and the template's "Changes Made" section is still empty.

CI is fully green (lint, CodeQL, unit tests, all e2e including the new slack-initiation job — note that job cannot exercise the DM reply path, which is why the blocker got past it).

[amithad]

[blocker] The DM path never resolves replies — binding the DM channel id can't match what the inbound handler resolves, so the example's headline scenario ("When Monroe replies to the DM, the reply resolves … to the initiated session") silently fails into a context-less session.

• slack_chat.py derives the inbound key as thread_ts = body.get("thread_ts") or body["ts"] and calls resolve_session_id(thread_ts) — the channel id is never passed to resolution.
• So after binding D999: a top-level DM reply resolves its own ts (miss), and a thread-reply resolves the bot message's ts (also a miss — the mapping key is the channel id). No DM reply style ever hits the mapping.
• This violates the round-trip rule this PR itself adds to ak-dev-new-messaging-integration/SKILL.md: "The returned id MUST equal the id your inbound webhook derives from a reply (before resolution)".
• The channel case (target = C…) is correct: bind response["ts"], thread-replies resolve it. Only DMs are broken.
• Suggested fix (core, preferred): in slack_chat.handle(), when body.get("channel_type") == "im" and the thread_ts resolution came back unchanged, also try self.resolve_session_id(channel) — then the DM-channel binding works for every reply style, and DM continuity holds. Alternative (example-only): return response["ts"] for DMs too and document that recipients must reply in-thread — fragile, since the natural DM reply is top-level.
• Whichever way: update server_test.py:125 (it currently asserts the wrong assumption, thread_id == "D999" # DM replies arrive keyed by the DM channel id) and add a test that exercises the inbound DM-reply resolution — the current suite only covers the outbound half, which is how this slipped past a green CI.