Open
Conversation
…tartup warning, secret catalog
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)None. Files Reviewed (17 files)
Reviewed by gpt-5.4-20260305 · 3,344,263 tokens |
…upstream responses
…abort guard, version test)
…base cast
Extract makeClient() helper to eliminate repeated KiloChatClient construction
across the three outbound handlers. Narrow the outbound.base cast from `as never`
to `as { deliveryMode: 'direct'; attachedResults: unknown }` so deliveryMode
remains type-checked. Add FRAGILE comment documenting the SDK-spread dependency.
* feat(kilo-chat): scaffold service with wrangler, vitest, drizzle
* feat(kilo-chat): add ULID generation utility
* feat(kilo-chat): add SSE formatting helpers
* feat(kilo-chat): add Drizzle schemas for ConversationDO and MembershipDO
* feat(kilo-chat): add dual auth middleware (JWT + API key)
* feat(kilo-chat): add MembershipDO with conversation index
Per-user/bot Durable Object storing a conversation membership list with
CRUD ops (add, list, update lastMessageId, remove). Includes vitest tests
and updated wrangler types.
* feat(kilo-chat): add ConversationDO with message CRUD
* feat(kilo-chat): add conversation routes (create, list, get)
Implements POST /v1/conversations, GET /v1/conversations, and GET /v1/conversations/:id behind auth middleware, with full integration test coverage.
* feat(kilo-chat): add message routes (create, list, edit, delete)
Implements POST /v1/messages, GET /v1/conversations/:id/messages,
PATCH /v1/messages/:id, and DELETE /v1/messages/:id with membership
checks, webhook queue enqueue for bot members, and MembershipDO
lastMessageId updates.
* feat(kilo-chat): add SSE events endpoint with fan-out and replay
- Add in-memory SSE client tracking and broadcast() to ConversationDO
- Broadcast message.created, message.updated, message.deleted events after each DB write
- Add fetch() handler on ConversationDO to handle /subscribe with member auth
- Support Last-Event-ID replay by querying messages with id > lastEventId
- Add alarm() keepalive that pings all connected clients every 30s
- Add /v1/conversations/:id/events route that forwards to DO's fetch handler
- Tests: access control (403/404), broadcast no-crash, streaming header checks, replay verification
- Streaming tests placed last in file to work around miniflare SQLite WAL isolated storage limitation
* feat(kilo-chat): add typing indicator endpoint with SSE broadcast
* feat(kilo-chat): add webhook queue delivery with HMAC signing
Implements the WEBHOOK_QUEUE consumer that delivers HMAC-SHA256 signed
payloads to the kiloclaw webhook endpoint, with per-message ack/retry
and graceful handling when secrets are not configured.
* fix(kilo-chat): resolve all oxlint errors
* refactor(kilo-chat): replace hand-rolled ULID with ulid package, use monotonicFactory for message IDs
* fix(kilo-chat): review fixes — webhook payload, timing-safe auth, SSE replay, writer cleanup
- Fix webhook queue message shape to match WebhookMessage type (was sending wrong fields)
- Use constant-time comparison for API key auth via crypto.subtle.timingSafeEqual
- SSE replay always sends message.created for missed messages (client never saw them)
- Close dead SSE writers on disconnect to prevent resource leaks
- Remove redundant callerKind guard in message creation
* fix(kilo-chat): review round 2 — timing-safe fix, server-controlled versioning, single webhook
- Remove redundant string-length check in timingSafeEqual (keep byte-length only)
- Server now controls message version (increments from current), not client-supplied
- Send one webhook per message (not one per bot member) since payloads are identical
- Allow version 0 in edit schema for stale clients
* fix(kilo-chat): harden webhook error path against body read failure
* refactor(kilo-chat): rename KILOCHAT_API_KEY to KILOCHAT_API_TOKEN to match kiloclaw
* feat(kilo-chat): add chat.kiloapps.io custom domain route
* fix(kilo-chat): bind NEXTAUTH_SECRET to NEXTAUTH_SECRET_PROD in secrets store
* fix(kiloclaw/kilo-chat): update plugin to send content blocks instead of flat text
Replace `text: string` with `content: ContentBlock[]` in CreateMessageParams and
EditMessageParams, wrap text strings in `[{ type: 'text', text }]` blocks at all
call sites (preview-stream and webhook deliver), and update all three test files
to match. Also change the e2e script SANDBOX_ID default to 'e2e-test-sandbox'.
* fix(kilo-chat): add zod validation at all input boundaries
Convert kiloclaw worker export to WorkerEntrypoint class and add deliverChatWebhook RPC method. kilo-chat now calls kiloclaw directly via service binding instead of external HTTP, eliminating the need for HMAC signing/verification and the KILOCHAT_WEBHOOK_SECRET entirely. - Convert kiloclaw default export from plain object to WorkerEntrypoint - Add deliverChatWebhook RPC: resolves instance from targetBotId, forwards to Fly machine - Enqueue one webhook per bot member (future-proofs multi-bot conversations) - Remove HMAC signing from kilo-chat and verification from plugin - Delete KILOCHAT_WEBHOOK_SECRET from all services, types, and config
Hardcode allowed origins (kilo.ai, app.kilo.ai, localhost:3000) with optional ALLOWED_ORIGINS env var override. Applied to all /v1/* routes. Also fix vitest config to properly stub the kiloclaw service binding.
getBotMembersExcluding returns { id, kind } objects, not strings.
The queue payload needs the string id, not the full object.
…erEntrypoint The RPC refactor (4b223d9) removed KILOCHAT_WEBHOOK_SECRET and converted the worker default export to WorkerEntrypoint, but left three test-only leftovers that broke CI: - catalog.test.ts: asserted the removed secret was still in INTERNAL_SENSITIVE_ENV_VARS. - config-writer.test.ts: stubbed the removed secret in env fixture. - index.test.ts: cloudflare:workers mock did not export WorkerEntrypoint, so importing src/index.ts threw at module load.
Introduce the reactions table with message/member foreign keys and the
invariant CHECK on deleted_at/removed_id. Tighten the existing tables
with FKs (messages.sender_id, messages.in_reply_to_message_id,
conversation.created_by) and CHECKs (members.kind, messages.deleted,
messages.version). Pre-deploy: migration is regenerated from scratch.
Also catch constraint violations in initialize() and createMessage()
to return {ok: false} results instead of throwing — prevents isolated
storage corruption in the CF vitest pool. Swap insert order in
initialize() so members are created before conversation (FK).
Commit fdee92c changed ConversationDO.initialize() from void to a result type so that constraint violations no longer poison the DO storage. The route forgot to check the result — constraint failures during init were silently swallowed, creating broken conversations. Also drop a redundant/mislabeled schema-constraint test.
Implements the single-table write rules: new inserts, idempotent re-adds on live rows, soft-delete on remove, and re-activation with a fresh id on re-add.
…ests Shift AddReactionResult and RemoveReactionResult to an ok-primary discriminated union matching CreateMessageResult. Lets consumers use a single narrowing pattern (`if (!result.ok) ...`) without mixing `ok in result` and `added in result` checks. Also tightens the FK rejection tests to assert the error branch explicitly.
listMessages now returns reactions grouped by emoji with count and memberIds per message. Dead (removed) reactions are excluded. Uses the partial index reactions_by_message_live so aggregation is an O(live rows) scan.
Extends the /subscribe replay path to include reaction.added and reaction.removed events alongside messages, ordered by ULID. Uses UNION ALL to guarantee index usage on both the id and removed_id columns. For re-add cycles, replay is lossy — only the final add event survives in storage.
The SSE replay tests in Task 4 triggered workerd TransformStream unhandled rejections that were masked by globally enabling dangerouslyIgnoreUnhandledErrors. That flag suppresses unhandled rejections across the whole suite and would hide real bugs in unrelated tests. Root cause: the collectReplay helper used Promise.race(reader.read(), timeout) in a loop. When the timeout won (after all replay data was consumed), a pending reader.read() promise was left dangling. The subsequent reader.cancel() caused that orphaned read to reject inside workerd's RPC/TransformStream plumbing as an unhandled "Stream was cancelled" error that cannot be caught from JS. Fix: rewrite collectReplay to read the single pre-buffered replay chunk directly (no race), then cancel the stream while no read() is pending. This mirrors how the existing events-routes.test.ts replay test works — it reads until the expected content appears, breaking the loop before cancelling, so there is never a dangling read promise.
The live broadcast path includes `at` on reaction.added, but the /subscribe replay was dropping it. Spec requires it on replay too. Select added_at in the UNION ALL and emit it in the SSE payload.
Adds the HTTP surface for reactions. Emoji validation enforces 1-64 UTF-8 bytes and rejects C0/C1 control chars. Member-of-conversation gate mirrors the existing /v1/messages routes.
POST and DELETE /_kilo/kilo-chat/messages/:id/reactions proxy to the kilo-chat service with the sandbox-scoped API token. Mirrors the existing /send, /messages edit/delete, /typing proxy pattern.
Extend KiloChatClient with HTTP wrappers that hit the new controller
reaction proxy routes. addReaction accepts both 201 (fresh) and 200
(dedupe) and returns { id }; removeReaction expects 204.
Implements handleAction for the shared message tool with emoji shortcode normalization via node-emoji. Routes add/remove through the existing KiloChatClient to the controller proxy. Registered in channel.ts alongside the existing sendText outbound.
Replace Object.assign + as-any cast with a straightforward spread, matching the WhatsApp reference pattern. Drop an unrequested capabilities declaration that was added outside the plan's scope.
…in handleAction Replaces a hand-rolled inline type in channel.ts with the canonical SDK type. Future SDK changes to the context shape will surface at compile time rather than silently diverging.
Maps KILOCHAT_REACTION_LEVEL env to config.channels['kilo-chat'].reactionLevel. Defaults to 'minimal' when unset or invalid. OpenClaw reads this value to guide agent reaction use; kiloclaw is pure passthrough.
Two pre-existing gaps made the reactions feature untestable end-to-end
through the real kilo-chat → kiloclaw → container pipeline.
1. deliverChatWebhook couldn't route to instance-keyed sandboxes.
The RPC entrypoint in services/kiloclaw/src/index.ts assumed the
legacy sandboxId format (base64url-encoded userId) and decoded the
incoming `bot:kiloclaw:<sandboxId>` target back to a userId for
registry lookup. For the current `ki_{uuid-no-hyphens}` instance-
keyed format this yielded Unicode garbage, so the registry lookup
always missed and webhooks silently dropped after 4 retries.
Detect the `ki_` prefix and resolve the DO directly by instanceId
(which is the doKey). Legacy format keeps the existing
registry+Postgres+userId-fallback chain.
2. Plugin had no messaging adapter.
With no `messaging` block, OpenClaw's outbound target resolver had
no way to parse `kilo-chat:<conversationId>` targets, so the
agent's `message` tool (including `action: "react"`) failed with
`Unknown target ...` before our handleAction even ran.
Add a minimal messaging adapter: normalize/parse the optional
`kilo-chat:` prefix, recognize ULID conversationIds, and build a
direct outbound session route. Mirrors the pattern used by
qa-channel / nostr.
Verified end-to-end locally: user message → kilo-chat queue →
env.KILOCLAW.deliverChatWebhook → docker-local container →
agent replies and reacts → reaction.added arrives on SSE and
aggregates correctly in GET /v1/conversations/:id/messages.
…rom empty emoji When 'emoji' was missing and 'remove' was not provided, the action silently routed into the delete branch and threw 'remove requires a specific emoji', making a forgotten emoji indistinguishable from an explicit remove request. Now 'remove' defaults to false when absent, and missing/empty emoji on the add path throws 'emoji is required'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the kilo-chat channel: a kiloclaw OpenClaw plugin and the external chat backend service it talks to, with Telegram-style live-edit streaming and message reactions.
kilo-chat plugin (
services/kiloclaw/plugins/kilo-chat/)OpenClaw channel plugin for kiloclaw that talks to the kilo-chat service.
Outbound (five actions via controller proxy):
POST {KILOCHAT_BASE_URL}/v1/messages— create a message; returns{ messageId, version }.PATCH {KILOCHAT_BASE_URL}/v1/messages/:id— edit with monotonicversion;409is treated as a benign drop.DELETE {KILOCHAT_BASE_URL}/v1/messages/:id— preview cleanup on dispatch failure.POST {KILOCHAT_BASE_URL}/v1/conversations/:id/typing— typing indicator (server holds ~5s, plugin re-pings every 3s).POST {KILOCHAT_BASE_URL}/v1/messages/:id/reactions/DELETE …— add/remove reactions, wired through the OpenClawreactmessage-tool action.All share the same auth hop: plugin → controller bearer
OPENCLAW_GATEWAY_TOKEN, controller re-auths withBearer KILOCHAT_API_TOKEN+x-kilo-sandbox-idon the upstream leg.Preview streaming: Inbound dispatch instantiates a per-conversation
PreviewStream. FirstonPartialReplytoken POSTs; subsequent partials coalesce into one PATCH per 500ms window.finalizeflushes pending edits, then performs one final PATCH. Dispatch failuresabortand DELETE the in-flight message.409drops no longer clobberlastSentText, so a subsequent flush/finalize re-sends.Reactions: Plugin exposes a
reactaction through the SDK'sChannelMessagingAdapter.actionshook.handleKiloChatReactActionresolvesconversationId/messageIdfrom tool params ortoolContext, normalises emoji shortcodes vianode-emoji(with GitHub/Slack aliases), and calls the controller add/remove endpoints.resolveExecutionModeislocalso the tool runs inline in the agent turn.Outbound target parsing:
messagingadapter implementsnormalizeTarget/parseExplicitTarget/resolveOutboundSessionRoutesokilo-chat:<conversationId>(ULID) works as an explicit send target.Inbound webhook: kilo-chat service delivers webhooks via CF service binding RPC → kiloclaw worker's
deliverChatWebhookmethod → resolves target Fly machine → forwards to controller → OpenClaw SDK dispatch with typing keepalive. Routing supports both legacybase64url(userId)sandbox IDs and newki_{uuid}instance-keyed sandbox IDs (via@kilocode/worker-utils/instance-id). No HMAC needed — the service binding is a trusted internal call.Message format: Uses
contentblocks ([{ type: "text", text: "..." }]) for extensibility.kilo-chat service (
services/kilo-chat/)Cloudflare Worker chat backend at
chat.kiloapps.io.Architecture:
ConversationDO(per-conversation state — messages, reactions, members, SSE fan-out, typing) andMembershipDO(per-user conversation index with denormalized recency)x-kilo-sandbox-idfor bot/service callersLast-Event-IDreplay (messages + reactions interleaved), 30s keepalive pings via DO alarms(message_id, member_id, emoji)with soft-delete + monotonic event IDs for replayAPI surface:
POST/v1/conversationsGET/v1/conversationsGET/v1/conversations/:idPOST/v1/messagesGET/v1/conversations/:id/messagesPATCH/v1/messages/:idDELETE/v1/messages/:idPOST/v1/messages/:id/reactionsDELETE/v1/messages/:id/reactionsGET/v1/conversations/:id/eventsPOST/v1/conversations/:id/typingData model: ULID message IDs (monotonic via
ulidpackage), JSON content blocks, server-controlled versioning for edits (409 on stale), soft deletes, reactions withadded_at/removed_idpair enforced by CHECK constraint. Supports multi-party conversations (data model ready, currently 1:1 user + bot).Wiring
buildEnvVarspassesKILOCHAT_API_TOKEN(encrypted) +KILOCHAT_BASE_URL(plain) +KILOCHAT_REACTION_LEVEL(plain); secret catalog registersKILOCHAT_API_TOKEN;config-writerenableschannels['kilo-chat']andplugins.entries['kilo-chat']when bothKILOCHAT_API_TOKENandKILOCHAT_BASE_URLare set, and threadsreactionLevelthrough; Dockerfile builds + installs@kiloclaw/kilo-chatat/usr/local/lib/node_modules/@kiloclaw/kilo-chat; CI content-hash includesplugins/kilo-chat/.Kiloclaw worker converted to
WorkerEntrypointclass to support RPC. kilo-chat service binds to kiloclaw viaservicesbinding in wrangler.jsonc.Test plan
pnpm testinservices/kiloclaw— all passpnpm testinservices/kiloclaw/plugins/kilo-chat— 67 tests (PreviewStream, client, react-action, channel adapter, webhook)pnpm testinservices/kilo-chat— 88 tests (DO unit, reactions, routes, SSE replay, webhook delivery)pnpm typecheckin both services — cleanpnpm lintin both services — 0 errorsreacttool,reaction.addedevent fires and is visible in the aggregated GET (seeplugins/kilo-chat/LOCAL_E2E.md)