Skip to content

feat(mobile): PR 5b — kilo-chat presence, events, shared hooks#2950

Merged
iscekic merged 60 commits intofeat/kilo-chat-migration-pr1from
feat/kilo-chat-migration-pr5b
Apr 30, 2026
Merged

feat(mobile): PR 5b — kilo-chat presence, events, shared hooks#2950
iscekic merged 60 commits intofeat/kilo-chat-migration-pr1from
feat/kilo-chat-migration-pr5b

Conversation

@iscekic
Copy link
Copy Markdown
Contributor

@iscekic iscekic commented Apr 30, 2026

Summary

Phases 9–11 of the mobile kilo-chat migration (plan). Adds the presence/event subscription primitives the mobile app needs to follow user attention, and extracts the platform-agnostic data hooks (useConversations, useMessages and friends) from web into a new shared @kilocode/kilo-chat-hooks package consumed by both web and mobile.

After this lands the primitives are in place but nothing is rendered yet — the UI components and routes ship in PR 5c and 5d.

What's in here

  • Phase 9 — Mobile presence subscriptions (Tasks 30–33): usePresenceSubscription primitive, plus useAppPresence (mounted in (app)/_layout.tsx), useInstancePresence, and useConversationPresence. The instance/conversation hooks gate on both AppState === 'active' and expo-router focus so we only hold a presence subscription while the user is genuinely on that surface.
  • Phase 10 — Mobile event subscriptions (Tasks 34–35): useEventSubscription primitive that wires through EventServiceClient.on(eventName, …) for a list of event names (the plan's snippet was schematic — the actual client doesn't have a wildcard listener). useInstanceEventSubscription invalidates the conversations / bot-status query keys when their kiloclaw-instance events fire.
  • Phase 11 — Shared @kilocode/kilo-chat-hooks package (Tasks 36–38):
    • New package containing KiloChatHooksProvider, useKiloChatClient/useEventServiceClient, plus the relocated useConversations/useMessages hooks and helpers from web.
    • useMessages was extracted in three commits (37a/b/c) so each diff stays reviewable: base infinite query + optimistic send → edit/delete/react/exec mutations → useMessageCacheUpdater (live event-stream cache patcher).
    • The shared package is platform-agnostic: it does not import sonner. The toast call inside the action-failed handler became an injectable onActionFailed?: (msg: string) => void callback. Web's MessageArea now passes a sonner toast at the call site.
    • Mobile kilo-chat-provider.tsx now mounts KiloChatHooksProvider alongside the existing KiloChatContext, so the new shared hooks resolve their clients from the same instances that mobile already constructed.
    • Mobile useMarkRead adapter calls trpc.user.markChatRead and applies the returned badgeCount to the OS badge via expo-notifications.

Plan deviations worth flagging in review

  • The plan's useEventSubscription snippet calls eventService.on('event', message => …) as if there were a wildcard listener. There is not — the client routes by event name. The primitive now takes a readonly string[] of event names. useInstanceEventSubscription passes the explicit list (conversation.created, conversation.left, message.*, bot.status).
  • The plan described a unified useMessages({ sandboxId, conversationId }) returning { messages, sendMessage, editMessage, … }. Web's existing API is per-hook (useMessages, useSendMessage, useEditMessage, …). Refactoring to the unified shape would have churned every web call site for no behavior change, so the shared package preserves the per-hook API. Mobile re-exports match.
  • useMarkRead: the plan referenced result.totalBadge. The actual markChatRead procedure returns { badgeCount: number } — used that. tRPC client import path is @/lib/trpc (single file), not @/lib/trpc/use-trpc.
  • Per-package tsconfig (the repo has no tsconfig.base.json); modeled packages/kilo-chat-hooks/tsconfig.json after packages/event-service/tsconfig.json, adding "jsx": "react-jsx".

Stack

Test plan

  • Web: cd apps/web && pnpm typecheck && pnpm test clean
  • Shared package: cd packages/kilo-chat-hooks && pnpm typecheck clean
  • Mobile: cd apps/mobile && pnpm typecheck && pnpm lint — only the 5 pre-existing errors in chat.tsx/use-unread-counts.ts remain (those land in the Stream rip in PR 6)
  • Web smoke: send a message in tab A, verify it appears live in tab B (exercises useMessageCacheUpdater after extraction)
  • Web action-failed toast still appears (sonner is wired at the MessageArea.tsx call site now)

iscekic added 30 commits April 29, 2026 17:28
The badge_counts.badge_bucket column is a free-form string. To prevent
namespace collisions as more surfaces start emitting badge updates
(per-instance today, per-conversation later), centralize bucket-key
derivation in @kilocode/notifications and route NotificationChannelDO
through it. Mirrors the presence-context builders in @kilocode/event-service.

Safe to introduce now without a data migration because PR 2's migration
already wipes badge_counts.
…-chat producer

Adds kiloclawInstanceContext and kiloclawConversationContext path
builders to @kilocode/event-service, replacing hardcoded template
literals in kilo-chat's event-push.ts and its test so all callers
share a single source of truth.
When a chat message is persisted, fire-and-forget a call to
NOTIFICATIONS.sendPushForConversation so non-sender human members of the
conversation receive a push. Runs after realtime/event-service delivery
inside postCommitFanOut, with errors swallowed so push failures cannot
fail the send.

- Skip when there are no other human recipients or no sandboxId.
- senderUserId = callerId for human senders, null for bot senders.
- title is "<sandboxLabel> · <conversationTitle>"; bodyPreview is the
  first 200 chars of the concatenated text blocks.
- Add @kilocode/notifications workspace dep and layer the RPC method
  shape into Env via bindings.d.ts.
- Add a notifications-stub worker to the vitest config so tests can
  spy on env.NOTIFICATIONS.sendPushForConversation, and globally mock
  sandbox-lookup in setup.ts (it imports pg via @kilocode/db).
…es, fix test mock

- Remove `stream-chat` from `services/notifications/package.json`; the Stream
  webhook (its only consumer) was deleted earlier in the stack.
- Regenerate `worker-configuration.d.ts` so the workerd runtime types match the
  current toolchain (sibling services were on `1.20260312.1`; this one had
  drifted to `1.20251217.0` from a stale local cache).
- Fix the global test mock to reference the renamed `badge_counts` table; the
  setup file was authored against the pre-rename name and never matched.
- Tidy two pre-existing lint nits in the new test files (`import type` for
  type-only import, drop unused `cols` parameter).
…leak

- Switch `NotificationsService` from default-only to a named class export
  with a separate default. `services/kilo-chat/wrangler.jsonc` binds via
  `entrypoint: "NotificationsService"`, which resolves named module
  exports. The default-only form (`export default class NotificationsService`)
  exports under the `default` key — kilo-chat's RPC binding would not have
  resolved at deploy. Mirrors the existing pattern in
  `services/kilo-chat/src/index.ts` (`KiloChatService`).

- `dispatchPush` now uses a two-stage idempotency record (`pending` →
  `delivered`). The badge increment was previously non-idempotent: an
  Expo failure returned `failed` without writing the idempotency key, so
  upstream retries (which the design explicitly invites) re-ran the
  increment before the next send and inflated the badge by one per
  retry. The `pending` marker is written before the increment and
  short-circuits the increment on retry; the `delivered` marker is only
  written on success.

- `setAlarm` is now gated on `getAlarm() === null`. Calling `setAlarm`
  unconditionally on each successful push — as the previous code did —
  replaces the pending alarm and pushes the cleanup forward indefinitely
  on a conversation receiving more than one push per `IDEM_TTL_MS`,
  leaking expired idempotency entries.

Adds two test cases covering the badge-retry and alarm-reset paths.
- Schedule the cleanup alarm when writing the `pending` marker, not only
  on `delivered`. Without this, an Expo failure followed by no further
  push activity for the conversation leaves the `pending` record in DO
  storage forever (no alarm was ever set to prune it).

- After the alarm fires, reschedule for the earliest remaining record's
  expiry instead of leaving the alarm slot empty. Otherwise a quiet
  conversation strands its younger entries until some unrelated future
  dispatch wakes the DO up.

Both paths go through a small `ensureCleanupAlarm` helper that gates on
`getAlarm() === null` so a busy conversation still doesn't push the
alarm forward on every call.
The kiloclaw-scoped presence paths are literally `/presence` prefixed
onto the kiloclaw event-context paths. Build them by composition so the
`/kiloclaw/{sandboxId}[/{conversationId}]` segment shape is defined in
exactly one place — `kiloclaw-contexts.ts`.

Pure refactor; same string output, template-literal types still narrow
to the same shape.
Introduces a single app-shell EventServiceProvider that owns the
EventServiceClient and KiloChatClient for all authenticated routes.
Mounted in (app)/layout.tsx so platform/instance/conversation presence
subscriptions and the kilo-chat UI share one WebSocket.

KiloChatLayout now consumes the global clients via useEventServiceClient()
instead of spinning up its own pair, and the getToken prop is removed from
KiloChatLayoutProps (along with both call sites). The local
useEventService(getToken) factory is dead code and has been deleted;
useInstanceContext / useConversationContext stay since they take
EventServiceClient as a parameter.
Thin hook that subscribes the global EventServiceClient to a single
context for the lifetime of the calling component, gated by an `active`
flag. Will back upcoming platform- and instance-level presence
indicators.
…eSubscription

- Drop dead getToken field from KiloChatContextValue (no consumers).
- Remove useInstanceContext / useConversationContext hooks; both call
  sites now use the shared usePresenceSubscription primitive directly.
- Harden usePresenceSubscription against empty-string contexts.
- usePresenceSubscription: accept 'string | null' instead of empty-string
  sentinel; update call sites (KiloChatLayout, MessageArea, useInstancePresence)
- kilo-chat router: validate expiresAt with z.iso.datetime()
- kilo-chat-router test: verify the JWT payload (kiloUserId, tokenSource,
  version) and that expiresAt lands in the expected ~1h window
- MessageArea: comment distinguishing the always-on chat-event subscription
  from the visibility-gated presence subscription
Multiple consumers can now independently hold the same context without
trampling each other. The wire context.subscribe/context.unsubscribe
messages are only sent on the 0->1 and 1->0 refcount transitions; the
intermediate churn stays client-side.

Resubscribe-on-reconnect dedupes by context key.

Tests cover: double-subscribe collapses to a single wire send, partial
unsubscribe keeps the context alive, last-consumer-out releases it,
mixed batches only send newly-active contexts, unknown-context
unsubscribes are no-ops, and reconnect resubscribes each context once.
iscekic added 19 commits April 29, 2026 21:06
Hoist cache and in-flight promise refs to module scope so all
useKiloChatTokenGetter() instances (provider + useCurrentUserId) share
one cache instead of each maintaining an independent one.

Wrap the fetch in try/catch/finally: on error rejectShared() is called
so concurrent waiters fail fast instead of hanging forever, and
inFlight is always cleared in finally regardless of outcome.
- Key the module-level kilo-chat JWT cache and in-flight ref on the
  current auth token, so signing out and back in as a different user
  within the 1h token window no longer returns the previous user's
  cached JWT.
- Restructure dedup so the first caller awaits the same shared promise
  via a slot reference, eliminating the unhandled rejection that the
  prior resolve/reject-pair pattern produced when the only caller's
  fetch failed.
- Decode kiloUserId from the JWT payload instead of the standard `sub`
  claim — generateApiToken writes the user id as kiloUserId, so the
  sub-based version always returned null.
KiloChatProvider builds its EventService and KiloChat clients exactly
once via useState initializer, so it captures whatever getter exists at
first mount. Closing the previous getter over a render-time `authToken`
meant a cold start where the (app) layout mounted before SecureStore
finished loading would freeze the clients with an undefined token,
trapping them in a permanent reconnect loop.

Read the auth token from SecureStore inside the getter, the same pattern
trpcClient uses. The hook returns a stable callback with no React deps,
and the cache stays keyed on the auth token so user-switch safety is
preserved.
…send

Move PAGE_SIZE, helper functions (applyReactionAdded/Removed, restoreMessageInCache,
removeMessageFromCache, findMessageInCache), useMessages infinite-query hook, and
useSendMessage mutation into @kilocode/kilo-chat-hooks. Web's useMessages.ts re-exports
the moved hooks and retains local helper copies for remaining mutations (37b will collapse).
…kage

Moves the live event-stream cache patcher from the web-only useMessages
file into @kilocode/kilo-chat-hooks. Adds an optional onActionFailed
callback so platform wrappers inject toasts; web passes toast.error.
@iscekic iscekic self-assigned this Apr 30, 2026
…on API

- Add packages/kilo-chat-hooks/src/query-keys.ts with conversations/
  conversation/messages/bot-status helpers; route every hook + invalidator
  through it. Fixes the mobile useInstanceEventSubscription bug where
  invalidations used ['conversations', sandboxId] but the queries register
  under ['kilo-chat', 'conversations', sandboxId], so list previews and
  unread counts never refreshed on incoming events.
- useEventSubscription now takes a single event name; callers register one
  hook per event. Drops the events.join('|') dependency hack and the
  eslint-disable. useInstanceEventSubscription becomes six explicit
  registrations.
- Drop the hardcoded English toast string from useMessageCacheUpdater;
  onActionFailed is () => void and the message lives at each call site.
- Extract useAppActiveAndFocused to deduplicate AppState+focus boilerplate
  shared by useInstancePresence and useConversationPresence.
Comment thread apps/mobile/src/components/kilo-chat/hooks/use-instance-event-subscription.ts Outdated
Comment thread apps/mobile/src/components/kilo-chat/hooks/use-instance-event-subscription.ts Outdated
Comment thread apps/mobile/src/components/kilo-chat/hooks/use-instance-event-subscription.ts Outdated
@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Apr 30, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • apps/mobile/src/components/kilo-chat/hooks/use-instance-event-subscription.ts

Reviewed by gpt-5.5-2026-04-23 · 522,449 tokens

The instance-level subscription was listening for message.created/updated/
deleted, which are published on conversation contexts and never fire here.
Replace them with conversation.renamed, conversation.read, and
conversation.activity — the events kilo-chat actually pushes to the
instance context — so list updates (title, unread, last-activity)
invalidate the conversations query as intended.
Base automatically changed from feat/kilo-chat-migration-pr5a to feat/kilo-chat-migration-pr1 April 30, 2026 13:20
@iscekic iscekic merged commit 9dfe27a into feat/kilo-chat-migration-pr1 Apr 30, 2026
1 check passed
@iscekic iscekic deleted the feat/kilo-chat-migration-pr5b branch April 30, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant