Conversation
Summary
Tests
|
|
Implementation complete for #58 — all 6 items. Build ✅, typecheck ✅, lint ✅, unit tests ✅. E2E running. Requesting review. |
noa-lucent
left a comment
There was a problem hiding this comment.
Clean PR overall — the removal of runs/containers/panel code is thorough, the useUpdateChat mutation with optimistic updates is well-structured, and the WebSocket notification hook follows the existing graphSocket patterns correctly.
One major issue to address:
useChatNotificationsregisters arun_status_changedhandler that invalidates a query key (['chats', chatId, 'runs']) that no longer exists after this PR's cleanup. This is dead code and should be removed.
Minor / nit items:
page.chats ?? []fallback inupdateChatPagesis unnecessary — boundary normalization already guaranteeschatsis an array.- Extra blank line in
Chats.tsx. - Three
chat.idreset effects inChatDetailHeadercould be consolidated.
Note (not in diff, but worth a follow-up): src/api/types/chat-resources.ts still exports RunMeta, RunMessageItem, JsonValue, and JsonPrimitive which are no longer referenced anywhere in the codebase after the run-related file deletions. Consider removing these dead types in this PR or a follow-up.
Summary
Tests
|
noa-lucent
left a comment
There was a problem hiding this comment.
All 4 review comments addressed and verified in code, plus the bonus cleanup of dead types from chat-resources.ts. Everything looks good — approving.
Summary
Tests
|
Summary
Tests
|
noa-lucent
left a comment
There was a problem hiding this comment.
New changes verified:
DEFAULT_TEST_INIT_IMAGEcentralizes the init image string — eliminates the pinned0.13.0magic string and keeps all e2e agent creation calls consistent.data-testid="chat-detail-header-agent"correctly separates the agent name testid from the summary area (chat-detail-header-title), improving test specificity.- All e2e test assertions updated to match.
Clean and straightforward. Approving.
Summary
Tests
|
Summary
Tests
|
Summary
Tests
|
noa-lucent
left a comment
There was a problem hiding this comment.
New commits verified:
createMembership/acceptMembershiphelpers — follow the establishedpostConnect+ typed wire response pattern consistently with existing helpers. Clean.- Un-skipped shared chat test — properly sets up org membership before creating the chat, correctly using User A's context for
createMembershipand User B's foracceptMembership. organization-switching.spec.tsfix — the oldbase.skipwas misleading (the!signedIncase means "already authenticated", not "MockAuth disabled"). The{ force: true }retry correctly clears stale auth state and re-authenticates as the fresh no-orgs user.- No remaining
test.skipacross all e2e specs — dead test code fully cleaned up.
Approving.
Summary
Testing
#58