AK-134 - Conversation Initiation from Agent - #362
Conversation
amithad
left a comment
There was a problem hiding this comment.
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:lineclaim checks out againstdevelop: the integration session-id derivation points (Slack/WhatsApp/Telegram/Messenger/Instagram/Teams/Gmail), queue pipeline citations, response-handler behavior,threadconfig gate,ConversationThreadManagersignatures (first_prompt,append_message,RLock),SQSHandler.send_message_to_output_queuekwargs,SessionStoreBuilder's valkey ImportError pattern, the Terraform response-store pattern, and the negative grep-claims (resolve_session_idunused inak-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 sharedcore/util/driver/drivers instead of writing new per-module thin drivers; theRedisDrivercitation 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 ondevelop.[nit]s:thread_idnaming 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, whileak-dev-write-specexpectsdesign.mdto survive its review cycles beforespec.mdis 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 adocs: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 atconfig.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
left a comment
There was a problem hiding this comment.
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 resolvesthread_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 fromdocs/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 updatingdocs/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") andadvanced/queue-mode-guide.md'sECSOutputConsumerdescriptions 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).
…annel-id fallback"" This reverts commit a892a46.
amithad
left a comment
There was a problem hiding this comment.
Re-review of the delta since my last review (commits c0f8adee…6780fa89).
All three previous findings are resolved:
[blocker]DM flow — resolved via the explicit-threading contract:send_initiation_messagenow returns the posted message'stsfor DMs and channels alike, so the round-trip invariant (returned id == inbound-derived id for a threaded reply) holds. The threading requirement and its rationale are documented consistently in server.py, the README (with a realistic dialog), the docs page, and spec.md. The commit history shows the channel-id fallback was tried and deliberately rejected — rightly so, in hindsight: with save-if-absent binding, a channel-id mapping would pin that DM to the first initiated session forever and misattribute any later initiation to the same person. The un-threaded-reply-starts-a-new-session trade-off is a conscious, honestly documented product decision.[suggestion]sidebar —'advanced/conversation-initiation'added todocs/sidebars.js.[nit]spec IAM wording — now describes the separate count-gated policy as built.
New findings on the delta (1 suggestion, 2 nits — inline):
[suggestion]ak-py/tests/test_slack_session_resolution.py:77— the new tests never invokehandle(), so the regression guard doesn't actually guard the code path it documents.[nit]examples/api/slack-initiation/server.py:120—PrintSessionContextHookdumps the full session item list to stdout on every notifier run, and the README never mentions it; reads as leftover debug output.[nit]docs/specs/ak-134/spec.md:176—slack_chat.py:130went stale (the import reformat moved the resolve call to:138).
Still open from earlier (minor, not re-anchored): plan iteration 10.2's docs/docs/deployment/ pages (aws-containerized.md:143, queue-mode-guide.md) still describe the output consumer without the INITIATION exception; and the PR title is still not conventional-commit form (feat: … (AK-134)) — a squash-merge title can cover that.
CI is fully green on the new head, including the new test_slack_session_resolution.py in unit tests.
amithad
left a comment
There was a problem hiding this comment.
Re-review of the delta since my last review (2a49f168, 0cb006d1).
All three previous findings are resolved — no new findings.
[suggestion]handle()-level tests — resolved exactly as suggested: the newTestHandleSessionResolutiondriveshandle(body, say)end-to-end with realistic DM event bodies (preset_bot_id, fake asyncsay, monkeypatchedslack_chat.AgentService) and covers all three cases: threaded reply with boundthread_ts→ mapped session; un-threaded reply → its ownts; channel-id-bound mapping never consulted byhandle(). This one would fail if a channel-id fallback were reintroduced — the guard now guards. TheFakeAgentService.agent = Noneearly-exit trick keeps the test minimal without mockingrun_multi; nice touch. The old direct-resolve tests are honestly reframed as a lower-level supplement.[nit]PrintSessionContextHook— resolved on both fronts: the README's "Try it" section now tells the reader to watch for the[SESSION-CONTEXT-DEMO]line, and the output is a one-line summary (turn count + roles, no message content).[nit]stale citation — fixed (slack_chat.py:138).
The implementation is consistent with the spec set and Agent Kernel conventions; CI is fully green on the new head, including the new async handle() tests.
Two minor items remain from earlier reviews (not re-anchored, just for the merge checklist): the docs/docs/deployment/ pages still describe the output consumer without the INITIATION exception (plan iteration 10.2), and the PR title isn't conventional-commit form — a squash-merge title like feat: agent-initiated conversations via messaging platforms (AK-134) covers the latter.
amithad
left a comment
There was a problem hiding this comment.
Re-review of the delta since my last review (96908b77…1ecd6062, excluding the develop merge content).
The two new queue-deployment examples close the biggest remaining gap — the process_message override contract that until now existed only in docs/tests is demonstrated end-to-end on both queue shapes (three-Lambda serverless, two-service ECS). Verified against the spec's user contract:
- Both response handlers implement the documented steps with the correct per-class record shape (
Bodyboto3 /bodyLambda), send →complete(), and a manager-None guard; the stock-handler drop is avoided exactly as designed. - The routing-context recipe (channel/
thread_tsas custom SQS attributes, carried by_get_record_attributes/_send_to_output_queueoverrides) is clean, and — as the plan.md note now records — required no library changes; every piece is a plain subclass of an existing extension point. TheAgentRequestAnyride-along claim checks out (BaseRunRequestisextra="allow",chat_service.py:119-130attaches extras). - Session resolution reuses the settled threading rule verbatim in both handlers, with the serverless one honestly documenting why it inlines the resolver instead of using the mixin.
- Test suites are comprehensive and mirrored across both deployments: inbound resolution (threaded/unthreaded), attribute passthrough, body ride-along, initiation delivery + bind, ordinary-reply delivery, missing-channel guard. The serverless Redis config follows the existing
scalable-openaiconvention, and the CI additions are exactly scoped (weekly-matrix registration + Slack TF vars). - plan.md records the deferred-then-completed queue examples; the docs page gained the routing recipe, runnable-examples list, and a deployment-scope section.
One finding (inline):
[nit]examples/aws-containerized/slack-initiation/slack_request_handler.py:95— the dedup-id fallback usesthread_ts(shared by every reply in a thread) instead of the message's ownts.
Forward-looking note (no action needed here): app_rest_service.py's docstring candidly flags that ECSIOHandler hardcodes its handlers and output consumer, forcing the example to replicate its wiring — ECSIOHandler.run(handlers=..., output_consumer=...) would be a worthwhile follow-up extension point.
CI is fully green on the new head.
There was a problem hiding this comment.
Pull request overview
This PR adds agent-initiated conversations to Agent Kernel: an agent can proactively message a user on a messaging platform, and the recipient’s reply is routed back into the same initiated session via a new Session ID Mapping mechanism. It includes core implementation (config + mapping stores + initiation tool/manager), wiring across REST + queue deployments, updated docs/specs, and runnable Slack examples for REST, AWS Lambda (serverless), and ECS (containerized).
Changes:
- Introduces
mapping_tableconfiguration +core/initiation/package (mapping stores,InitiationManager,InitiationMessage, andinitiate_conversationsystem tool). - Wires session-id resolution and initiation dispatch/guards into request handlers, agent runners, and response handlers (REST + queue deployments).
- Adds Slack initiation examples (REST + serverless + containerized), docs page + sidebar entry, and new/updated tests and skills.
Reviewed changes
Copilot reviewed 98 out of 103 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/aws-serverless/slack-initiation/README.md | New serverless Slack initiation example documentation (3-Lambda queue shape). |
| examples/aws-serverless/slack-initiation/pyproject.toml | Python deps for the serverless Slack initiation example. |
| examples/aws-serverless/slack-initiation/lambda_response_handler.py | Custom ResponseHandler to deliver INITIATION messages + bind mapping. |
| examples/aws-serverless/slack-initiation/lambda_request_handler.py | Slack webhook Lambda that resolves session_id and enqueues to input queue. |
| examples/aws-serverless/slack-initiation/lambda_deployment_test.py | Live endpoint smoke tests for deployed Slack events route/signature verification. |
| examples/aws-serverless/slack-initiation/lambda_agent_runner.py | Serverless agent runner carrying Slack routing attributes through SQS. |
| examples/aws-serverless/slack-initiation/deploy/variables.tf | Terraform variables for the serverless Slack initiation example. |
| examples/aws-serverless/slack-initiation/deploy/terraform.tfvars | Default tfvars for the serverless Slack initiation example. |
| examples/aws-serverless/slack-initiation/deploy/outputs.tf | Terraform outputs (agent_invoke_url). |
| examples/aws-serverless/slack-initiation/deploy/main.tf | Serverless module wiring + custom Slack gateway endpoint. |
| examples/aws-serverless/slack-initiation/deploy/Dockerfile.response_handler | Lambda image Dockerfile for response handler. |
| examples/aws-serverless/slack-initiation/deploy/Dockerfile.request_handler | Lambda image Dockerfile for request handler. |
| examples/aws-serverless/slack-initiation/deploy/Dockerfile.agent_runner | Lambda image Dockerfile for agent runner. |
| examples/aws-serverless/slack-initiation/deploy/deploy.sh | Builds dist folders and runs Terraform apply for the example. |
| examples/aws-serverless/slack-initiation/config.yaml | Example config enabling mapping_table + queue mode + redis stores. |
| examples/aws-serverless/slack-initiation/build.sh | Local build script for example dependencies via uv. |
| examples/aws-serverless/slack-initiation/.gitignore | Ignores Terraform artifacts and dist outputs for the example. |
| examples/aws-containerized/slack-initiation/slack_request_handler.py | ECS Slack webhook handler using Bolt + enqueueing requests. |
| examples/aws-containerized/slack-initiation/slack_output_consumer.py | ECS output consumer delivering INITIATION + ordinary replies to Slack. |
| examples/aws-containerized/slack-initiation/README.md | Containerized Slack initiation example documentation (2-service ECS). |
| examples/aws-containerized/slack-initiation/pyproject.toml | Python deps for the containerized Slack initiation example. |
| examples/aws-containerized/slack-initiation/deploy/variables.tf | Terraform variables for the containerized Slack initiation example. |
| examples/aws-containerized/slack-initiation/deploy/terraform.tfvars | Default tfvars for the containerized Slack initiation example. |
| examples/aws-containerized/slack-initiation/deploy/outputs.tf | Terraform outputs for ECS example (queues, services, mapping table). |
| examples/aws-containerized/slack-initiation/deploy/main.tf | Containerized module wiring + gateway endpoint rewrite + mapping table env injection. |
| examples/aws-containerized/slack-initiation/deploy/Dockerfile.rest-service | REST service container image definition. |
| examples/aws-containerized/slack-initiation/deploy/Dockerfile.agent-runner | Agent runner container image definition. |
| examples/aws-containerized/slack-initiation/deploy/deploy.sh | ECS example packaging + terraform apply flow. |
| examples/aws-containerized/slack-initiation/deploy/.terraform.lock.hcl | Terraform provider lock for containerized example. |
| examples/aws-containerized/slack-initiation/config.yaml | Example config enabling mapping_table and queue-mode wiring for ECS. |
| examples/aws-containerized/slack-initiation/build.sh | Local build script for the ECS example via uv. |
| examples/aws-containerized/slack-initiation/app_rest_service.py | Runs FastAPI + output consumer as threads (mirrors ECSIOHandler wiring). |
| examples/aws-containerized/slack-initiation/app_deployment_test.py | Live endpoint smoke tests for deployed ECS Slack events route. |
| examples/aws-containerized/slack-initiation/app_agent_runner.py | ECS agent runner implementing the Slack initiation two-agent flow. |
| examples/aws-containerized/slack-initiation/.gitignore | Ignores terraform/dist artifacts for ECS example. |
| examples/api/slack-initiation/server.py | Single-process REST Slack initiation example (InitiationSender implementation). |
| examples/api/slack-initiation/server_test.py | Local tests for REST Slack initiation example (signing + initiation send). |
| examples/api/slack-initiation/README.md | Documentation for single-process REST Slack initiation example. |
| examples/api/slack-initiation/pyproject.toml | Python deps for REST Slack initiation example. |
| examples/api/slack-initiation/config.yaml | Example config enabling mapping_table for REST initiation example. |
| examples/api/slack-initiation/build.sh | Local build script for REST example via uv. |
| docs/specs/ak-134/plan.md | Implementation plan for AK-134. |
| docs/specs/ak-134/design.md | Design document for agent-initiated conversations (AK-134). |
| docs/sidebars.js | Adds “advanced/conversation-initiation” to docs sidebar. |
| docs/docs/deployment/aws-serverless.md | Documents response-handler override contract for INITIATION messages (Lambda). |
| docs/docs/deployment/aws-containerized.md | Documents output-consumer override contract for INITIATION messages (ECS). |
| docs/docs/advanced/conversation-initiation.md | New docs page describing config/tool/override contracts and examples. |
| ak-py/tests/test_slack_session_resolution.py | Tests Slack session-id resolution behavior w/ mapping_table. |
| ak-py/tests/test_queue_request_handler_resolve.py | Tests queue request handler rewriting session_id via mapping resolution. |
| ak-py/tests/test_initiation_tool.py | Tests initiate_conversation tool + dispatcher + REST sender wiring. |
| ak-py/tests/test_initiation_response_handlers.py | Tests INITIATION guard behavior + user override contract + queue dispatcher. |
| ak-py/tests/test_config.py | Asserts mapping_table defaults to None (feature disabled by default). |
| ak-py/src/agentkernel/skills/ak-add-capabilities/SKILL.md | Adds end-user capability guidance for agent-initiated conversations. |
| ak-py/src/agentkernel/integration/whatsapp/whatsapp_chat.py | Routes session_id derivation through resolve_session_id(). |
| ak-py/src/agentkernel/integration/telegram/telegram_chat.py | Routes session_id derivation through resolve_session_id(). |
| ak-py/src/agentkernel/integration/teams/teams_chat.py | Routes Teams conversation_id through resolve_session_id(). |
| ak-py/src/agentkernel/integration/slack/slack_chat.py | Resolves Slack thread_ts via resolve_session_id() before selecting agent. |
| ak-py/src/agentkernel/integration/messenger/messenger_chat.py | Routes sender_id through resolve_session_id(). |
| ak-py/src/agentkernel/integration/instagram/instagram_chat.py | Routes sender_id through resolve_session_id(). |
| ak-py/src/agentkernel/integration/gmail/gmail_chat.py | Adds SessionIdResolver mixin + resolves thread/session ids via mapping. |
| ak-py/src/agentkernel/deployment/common/queue_request_handler.py | Rewrites mapped session_id for queue REST endpoint and returns resolved id. |
| ak-py/src/agentkernel/deployment/aws/serverless/core/router/rest_lambda.py | Adds mapping-based session_id rewrite before enqueue in serverless router. |
| ak-py/src/agentkernel/deployment/aws/serverless/akresponsehandler.py | Guards INITIATION messages (drop + log) and avoids response-store writes on failure. |
| ak-py/src/agentkernel/deployment/aws/serverless/akagentrunner.py | Registers initiation queue dispatcher when creating ChatService. |
| ak-py/src/agentkernel/deployment/aws/core/initiation_dispatch.py | New Output Queue dispatcher for InitiationMessage with message_type=INITIATION. |
| ak-py/src/agentkernel/deployment/aws/containerized/akoutputconsumer.py | Guards INITIATION messages (drop + log) and avoids response-store writes on failure. |
| ak-py/src/agentkernel/deployment/aws/containerized/akagentrunner.py | Registers initiation queue dispatcher at ECS agent runner startup. |
| ak-py/src/agentkernel/core/tool.py | Registers initiate_conversation system tool when mapping_table is configured. |
| ak-py/src/agentkernel/core/initiation/tools.py | New initiate_conversation tool implementation (creates session + runs agent + dispatches). |
| ak-py/src/agentkernel/core/initiation/model.py | Defines InitiationMessage + INITIATION_MESSAGE_TYPE. |
| ak-py/src/agentkernel/core/initiation/mapping/valkey.py | Valkey mapping-store backend. |
| ak-py/src/agentkernel/core/initiation/mapping/redis.py | Redis mapping-store backend. |
| ak-py/src/agentkernel/core/initiation/mapping/redis_like.py | Shared redis-like mapping store implementation. |
| ak-py/src/agentkernel/core/initiation/mapping/in_memory.py | In-memory mapping store (tests/local). |
| ak-py/src/agentkernel/core/initiation/mapping/firestore.py | Firestore mapping-store backend. |
| ak-py/src/agentkernel/core/initiation/mapping/dynamodb.py | DynamoDB mapping-store backend. |
| ak-py/src/agentkernel/core/initiation/mapping/cosmosdb.py | CosmosDB mapping-store backend. |
| ak-py/src/agentkernel/core/initiation/mapping/base.py | Mapping-store ABC + record key conventions. |
| ak-py/src/agentkernel/core/initiation/mapping/init.py | Mapping-store builder (backend selection). |
| ak-py/src/agentkernel/core/initiation/manager.py | InitiationManager, SessionIdResolver mixin, and InitiationSender contract. |
| ak-py/src/agentkernel/core/initiation/init.py | Public exports for initiation package. |
| ak-py/src/agentkernel/core/config.py | Adds mapping_table config block and MappingTableConfig model. |
| ak-py/src/agentkernel/api/http.py | Registers in-process InitiationSender dispatcher for REST deployments. |
| ak-py/src/agentkernel/api/handler.py | Makes RESTRequestHandler inherit SessionIdResolver for resolve_session_id(). |
| ak-deployment/ak-aws/containerized/variables.tf | Adds conversation_initiation Terraform variable. |
| ak-deployment/ak-aws/containerized/README.md | Documents conversation_initiation and mapping table behavior. |
| ak-deployment/ak-aws/containerized/outputs.tf | Outputs mapping table name when enabled. |
| ak-deployment/ak-aws/containerized/iam.tf | Adds REST service IAM policy for session-id-mapping table. |
| ak-deployment/ak-aws/containerized/dynamodb.tf | Adds DynamoDB session-id-mapping table resource when enabled. |
| .github/workflows/integration-test-weekly.yaml | Passes Slack TF_VAR secrets for new Slack initiation infra. |
| .github/test-config.yaml | Adds API slack-initiation example to test config. |
| .github/integration-test-config.yaml | Adds weekly deploy/test coverage for ECS + Lambda slack-initiation examples. |
| .agents/skills/ak-dev-new-messaging-integration/SKILL.md | Updates messaging integration checklist to resolve session_id via mapping and optional InitiationSender contract. |
| .agents/skills/ak-dev-architecture/SKILL.md | Adds initiation feature overview + config section to architecture skill. |
Files not reviewed (2)
- examples/aws-containerized/slack-initiation/deploy/.terraform.lock.hcl: Generated file
- examples/aws-serverless/slack-initiation/deploy/.terraform.lock.hcl: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
amithad
left a comment
There was a problem hiding this comment.
Re-review of the delta since my last review (cec6bbc6…7bda103f, excluding develop-merge content). This delta is a substantial redesign, not just fixes: the config block was renamed to conversation_initiation: with auto-enable in queue mode, the mapping store's namespace is now derived from the session store's own settings (no separate table/prefix/TTL config), a bring-your-own dotted-path store was added (matching develop's new BYO convention), and Gmail + Telegram initiation examples joined Slack.
Resolved from earlier reviews: the dedup fallback now uses body["ts"]; PrintSessionContextHook was removed; the long-open docs/docs/deployment/ pages item is finally closed; and test_integration_session_resolution.py drives inbound resolution through every integration's own handler method (Teams honestly documented as the one gap) — beyond what I asked for. The rename/derivation/BYO redesign is propagated consistently through spec, docs, skills, tests, and all five examples, and the spec's Feature-gate/Config/Testing sections were faithfully re-aligned.
Findings (1 blocker, 1 suggestion, 1 question — inline):
[blocker]ak-deployment/ak-aws/containerized/dynamodb.tf:31— the Terraform mapping table name can never match what the app derives: the module injects…-session_storeas the session table name, so the app looks for…-session_store-id-mapping, while Terraform creates (and IAM grants)…-session-id-mapping. The provisioned table is unreachable by construction.[suggestion]ak-py/src/agentkernel/core/initiation/mapping/__init__.py:81— with queue-mode auto-enable, a BYO dotted-pathsession.type(legal since the develop BYO-stores change) makes thisAKConfigErrorraise lazily inside every inbound request instead of failing at startup or degrading to feature-off.[question]docs/specs/ak-134/spec.md:9— auto-enable means stock queue deployments now advertiseinitiate_conversationon every agent while their stock response handler drops the dispatched message: the tool reports success, nothing is delivered. Was this default-on trade-off weighed?
CI is fully green — note the containerized example uses a Redis session store, which is why the DynamoDB pairing bug doesn't surface in any test.
amithad
left a comment
There was a problem hiding this comment.
Re-review of 0f08c098 — all three findings from my last review are resolved, no new findings.
[blocker]Terraform table naming — fixed exactly as suggested and then some:name = "${local.dynamodb_memory_table_name}-id-mapping"pairs the table with what the module injects asAK_SESSION__DYNAMODB__TABLE_NAMEby construction,var.conversation_initiationgained a validation requiringcreate_dynamodb_memory_table = true(cross-variable validation is fine — the module pinsrequired_version >= 1.9.5), the README documents the derivation rationale, and the spec's Terraform bullet was rewritten to explain why a literal name would drift. The IAM grant follows the resource ARN automatically.[suggestion]lazyAKConfigError— implemented with the intent split: auto-enabled degrades to feature-off with a single cached WARNING naming the remedy (_auto_disabled, cleared byreset()), explicitenabled: truestill raises, andInitiationQueueDispatcher.register()now warms the manager so an explicit misconfiguration surfaces at container start (ECS) / first invocation (Lambda) instead of on a user message. The six new tests inTestUnbuildableMappingStorecover degrade, inbound-keeps-working, cache-once, explicit-still-raises (twice), and reset semantics — exactly the matrix that matters.[question]auto-enable trade-off — answered and turned into an explicit, numbered spec decision (Feature gate #2): the split-gate alternative was considered and rejected with stated reasoning, and the advertised-but-undeliverable failure mode is now documented at every surface an operator would hit it — the docs page warning box, both deployment docs pages, the containerized module README, and theak-add-capabilitiesskill ("Step 3 is mandatory, not a refinement").
One nuance, noted for the record rather than as a finding: the startup warm-up runs where register() runs (the agent-runner process), so in the REST/IO container an explicitly enabled feature with a broken store still surfaces on the first inbound request — that's the operator-asked-for-it case, and failing loud there is defensible; the auto-mode degrade covers the process either way.
CI is fully green. From my side the PR is in mergeable shape — the earlier minor notes (conventional-commit title at squash time) remain the only leftovers.
| tags = merge(var.tags, { Type = "ResponseStore" }) | ||
| } | ||
|
|
||
| # ---------- DynamoDB Session ID Mapping (agent-initiated conversations) ---------- |
There was a problem hiding this comment.
These comments are too verbose.
Description
The design for initiation of a conversation from agent, implementation and an api example
Type of Change
Related Issues
Fixes #
Relates to #
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes