[Feat] Team data model with SQLite persistence and Zustand store#262
[Feat] Team data model with SQLite persistence and Zustand store#262
Conversation
Signed-off-by: samzong <samzong.lu@gmail.com>
|
Hi @samzong, DetailsInstructions for interacting with me using comments are available here. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust data layer for managing teams within the application. By moving from volatile local state to a persistent SQLite-backed architecture, teams are now reliably saved and retrieved across sessions. The changes span the entire stack, including shared type definitions, backend database schema updates, IPC communication channels, and a centralized state management store, culminating in an improved user experience for team creation and management. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Deploying cpwa with
|
| Latest commit: |
4f2db4d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b2ae4ff4.cpwa.pages.dev |
| Branch Preview URL: | https://feat-team-data-model.cpwa.pages.dev |
Signed-off-by: samzong <samzong.lu@gmail.com> # Conflicts: # packages/desktop/src/renderer/platform/index.ts
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a team management system, featuring a new Zustand store, SQLite database schema updates, and a dedicated UI for team creation and management. The review feedback identifies several areas for improvement: implementing rollback logic for optimistic updates in the team store to maintain state consistency, adding foreign key cascade deletes for database integrity, and addressing a potential race condition in the agent fetching logic within the UI.
| createTeam: async (params) => { | ||
| const now = new Date().toISOString(); | ||
| const id = crypto.randomUUID(); | ||
| const team: Team = { | ||
| id, | ||
| ...params, | ||
| createdAt: now, | ||
| updatedAt: now, | ||
| }; | ||
| set((s) => ({ teams: { ...s.teams, [id]: team } })); | ||
| const res = await deps.persistTeam({ | ||
| id: team.id, | ||
| name: team.name, | ||
| emoji: team.emoji, | ||
| description: team.description, | ||
| gatewayId: team.gatewayId, | ||
| source: team.source, | ||
| version: team.version, | ||
| agents: team.agents, | ||
| createdAt: team.createdAt, | ||
| updatedAt: team.updatedAt, | ||
| }); | ||
| if (!res.ok) { | ||
| console.error('[team-store] persistTeam failed:', res.error); | ||
| } | ||
| return id; | ||
| }, |
There was a problem hiding this comment.
The createTeam action performs an optimistic update by adding the new team to the store before the persistence call completes. However, it lacks a rollback mechanism if deps.persistTeam fails. This can lead to the UI showing data that was never actually saved to the database.
References
- Ensure state consistency by handling failures in asynchronous persistence calls, especially when using optimistic updates.
| updateTeam: async (id, updates) => { | ||
| const existing = get().teams[id]; | ||
| if (!existing) return; | ||
| const now = new Date().toISOString(); | ||
| const updated: Team = { ...existing, ...updates, updatedAt: now }; | ||
| set((s) => ({ teams: { ...s.teams, [id]: updated } })); | ||
| const res = await deps.persistTeam({ | ||
| id: updated.id, | ||
| name: updated.name, | ||
| emoji: updated.emoji, | ||
| description: updated.description, | ||
| gatewayId: updated.gatewayId, | ||
| source: updated.source, | ||
| version: updated.version, | ||
| agents: updated.agents, | ||
| createdAt: updated.createdAt, | ||
| updatedAt: updated.updatedAt, | ||
| }); | ||
| if (!res.ok) { | ||
| console.error('[team-store] updateTeam persist failed:', res.error); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Similar to createTeam, the updateTeam action performs an optimistic update without a rollback strategy. If the database persistence fails, the store will remain in an inconsistent state relative to the actual data on disk.
| deleteTeam: async (id) => { | ||
| set((s) => { | ||
| const next = { ...s.teams }; | ||
| delete next[id]; | ||
| return { teams: next }; | ||
| }); | ||
| const res = await deps.deleteTeam(id); | ||
| if (!res.ok) { | ||
| console.error('[team-store] deleteTeam failed:', res.error); | ||
| } | ||
| }, |
There was a problem hiding this comment.
The deleteTeam action removes the team from the store optimistically. If deps.deleteTeam fails, the team should be restored to the store to maintain consistency.
|
|
||
| sqlite.exec(` | ||
| CREATE TABLE IF NOT EXISTS team_agents ( | ||
| team_id TEXT NOT NULL REFERENCES teams(id), |
There was a problem hiding this comment.
The foreign key constraint for team_id in the team_agents table is missing an ON DELETE CASCADE clause. While the IPC handler manually deletes related agents, enforcing this at the database level ensures data integrity and prevents orphaned records if deletions occur through other paths (e.g., raw SQL execution or future refactors).
References
- Maintain database integrity by using appropriate foreign key constraints and cascade behaviors. (link)
| useEffect(() => { | ||
| if (!open || !gatewayId) return; | ||
| setLoadingAgents(true); | ||
| setSelectedAgentIds(new Set()); | ||
| window.clawwork | ||
| .listAgents(gatewayId) | ||
| .then((res) => { | ||
| if (res.ok && res.result) { | ||
| const payload = res.result as { agents?: AgentInfo[] }; | ||
| setAgentCatalog(payload.agents ?? []); | ||
| } | ||
| }) | ||
| .catch(() => {}) | ||
| .finally(() => setLoadingAgents(false)); | ||
| }, [open, gatewayId]); |
There was a problem hiding this comment.
There is a potential race condition in this useEffect. If the gatewayId changes rapidly or the dialog is closed before the listAgents promise resolves, setAgentCatalog and setLoadingAgents will still be called for the stale request. Consider using an abort controller or a simple boolean flag to ignore results from outdated effect iterations.
References
- Handle race conditions in asynchronous effects to prevent state updates on unmounted components or stale data. (link)
1 participant
Summary
Add a full data layer for Teams: SQLite persistence via Drizzle ORM, IPC CRUD handlers, preload bindings, a Zustand store in core, and UI wiring in TeamsPanel. Teams are now persisted across app restarts.
Type of change
[Feat]new featureWhy is this needed?
TeamsPanel (#258) used
useStateto manage team data — everything was lost on refresh. This PR adds real persistence so teams survive app restarts, and introduces a gateway-bound agent selection flow for team creation.What changed?
TeamandTeamAgentinterfaces to@clawwork/sharedteamsandteam_agentsSQLite tables (Drizzle schema + raw SQL migration)data:teams-list,data:team-get,data:team-persist,data:team-deleteteam-store.tsin core with full CRUD + agent management actionsuseTeamStorehookTeamsPanelto store, replacinguseStateTeamCardto use sharedTeamtype (agents.lengthinstead ofmemberCount)CreateTeamDialogwith gateway selector and agent multi-selectArchitecture impact
docs/architecture-invariants.md: dependency direction (shared←core←desktop)Linked issues
Closes #258
Validation
pnpm lintpnpm testpnpm check:ui-contractpnpm check(full gate — 274 tests pass)Screenshots or recordings
No layout changes — existing TeamsPanel card grid and CreateTeamDialog are preserved. CreateTeamDialog now includes a gateway selector (shown only when multiple gateways exist) and an agent multi-select list with checkboxes.
Release note
Checklist
[Feat]