feat(stores): Implement Zustand state management system#41
Merged
Conversation
Implements comprehensive Zustand-based state management as specified in Issue #10. ## Stores Added: - **useAgentStore**: Multi-agent orchestration (Architect, Implementer, Reviewer, Tester) - **useProjectStore**: Repository and workspace state with Git integration - **useChatStore**: Human-agent collaboration with threads and approvals - **useCredentialStore**: API credential metadata (secrets in SecureStore) - **useUserStore**: User preferences, settings, and GitHub profile ## Features: - AsyncStorage persistence layer for app restart survival - Immer middleware for immutable state updates - DevTools integration for debugging - Typed selectors for optimal re-renders - Comprehensive test suite (46 tests passing) ## Architecture: - All stores use immer + persist + devtools middleware stack - Credential store only tracks metadata; actual secrets use expo-secure-store - Selectors exported alongside stores for component optimization - Full TypeScript types for all state and actions Closes #10 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds five new Zustand stores (Agent, Chat, Credential, Project, User) with types, selectors, actions, immer immutability, AsyncStorage persistence, devtools integration, and tests; updates Jest setup/config and adds an Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
- Add packages directory to biome ignore (NativeWind v2 compat code) - Add index.html to biome ignore (landing page CSS specificity) - Fix tsconfig.json formatting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/stores/__tests__/agentStore.test.ts`:
- Around line 193-204: The test uses a non-null assertion on taskId; initialize
taskId when declaring it to avoid the assertion and satisfy the linter. Change
the declaration of taskId (used with result.current.addTask and
result.current.completeTask) to a defined default (e.g., let taskId = ''; or
assign immediately from addTask) and then call completeTask(taskId, '...')
without the trailing ! so the calls to addTask and completeTask
(result.current.addTask / result.current.completeTask) no longer require a
non-null assertion.
- Around line 164-176: The test uses a non-null assertion on taskId; to satisfy
the noNonNullAssertion lint rule, initialize taskId instead of using "taskId!"
or change its type—e.g., replace "let taskId: string;" with "let taskId = '';"
(or "let taskId: string | undefined = undefined;" and adjust the expect to check
definedness) so you can remove the non-null assertion when comparing
result.current.agents[0].currentTaskId to taskId; update references to taskId
and keep the act(() => { taskId = result.current.addTask(...) }) and subsequent
expects unchanged except removing the "!".
In `@src/stores/chatStore.ts`:
- Around line 243-246: clearThread currently empties state.messages[threadId]
but doesn't touch the unread count, leaving badges for an empty thread; update
the clearThread handler (the clearThread function that calls set and mutates
state.messages[threadId]) to also reset the unread count for that thread by
setting state.unreadCount[threadId] = 0 (or state.unreadCounts[threadId] = 0 if
your store uses plural naming), and guard with a null/undefined check (e.g.,
ensure state.unreadCount exists) so clearing a thread always clears its unread
counter.
In `@src/stores/credentialStore.ts`:
- Around line 177-182: The invalidateAll setter currently only flips
credential.status to 'unknown' but leaves validation metadata, so update the
invalidateAll implementation (the set(...) callback that iterates
state.credentials) to also clear the validation fields by setting each
credential.lastValidatedAt and credential.expiresAt to null (or undefined per
your codebase convention) when you set status = 'unknown'; reference the
invalidateAll method, state.credentials array, and the lastValidatedAt/expiresAt
properties to locate and update the logic.
In `@src/stores/projectStore.ts`:
- Around line 127-135: When removing the active project in removeProject,
branches and recentCommits are left populated causing stale Git info; update the
removeProject handler to also clear state.branches and state.recentCommits when
state.activeProjectId === projectId (similar to how state.workspace and
state.fileTree are set to null) so that state.branches = [] and
state.recentCommits = [] (or null if your store uses null) and thereby fully
reset branch/commit state for the removed active project.
🧹 Nitpick comments (2)
src/stores/agentStore.ts (1)
127-142: Use.substring()instead of deprecated.substr().
String.prototype.substr()is deprecated. Use.substring(2, 11)instead.♻️ Suggested fix
addTask: (task) => { - const taskId = `task-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + const taskId = `task-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; set((state) => {src/stores/__tests__/agentStore.test.ts (1)
215-215: Remove unused variable.
_stateis declared but never used. Consider removing it.♻️ Suggested fix
it('selectAgents should return all agents', () => { - const _state = useAgentStore.getState(); useAgentStore.setState({
Comment on lines
+164
to
+176
| let taskId: string; | ||
| act(() => { | ||
| taskId = result.current.addTask({ | ||
| agentId: 'test-agent-1', | ||
| description: 'Implement feature X', | ||
| status: 'pending', | ||
| }); | ||
| }); | ||
|
|
||
| expect(result.current.tasks).toHaveLength(1); | ||
| expect(result.current.tasks[0].description).toBe('Implement feature X'); | ||
| expect(result.current.agents[0].currentTaskId).toBe(taskId!); | ||
| }); |
Contributor
There was a problem hiding this comment.
Fix non-null assertion to resolve lint failure.
The pipeline fails due to noNonNullAssertion lint rule. Initialize taskId with a default or refactor to avoid the assertion.
🔧 Suggested fix
- let taskId: string;
+ let taskId = '';
act(() => {
taskId = result.current.addTask({
agentId: 'test-agent-1',
description: 'Implement feature X',
status: 'pending',
});
});
expect(result.current.tasks).toHaveLength(1);
expect(result.current.tasks[0].description).toBe('Implement feature X');
- expect(result.current.agents[0].currentTaskId).toBe(taskId!);
+ expect(result.current.agents[0].currentTaskId).toBe(taskId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let taskId: string; | |
| act(() => { | |
| taskId = result.current.addTask({ | |
| agentId: 'test-agent-1', | |
| description: 'Implement feature X', | |
| status: 'pending', | |
| }); | |
| }); | |
| expect(result.current.tasks).toHaveLength(1); | |
| expect(result.current.tasks[0].description).toBe('Implement feature X'); | |
| expect(result.current.agents[0].currentTaskId).toBe(taskId!); | |
| }); | |
| let taskId = ''; | |
| act(() => { | |
| taskId = result.current.addTask({ | |
| agentId: 'test-agent-1', | |
| description: 'Implement feature X', | |
| status: 'pending', | |
| }); | |
| }); | |
| expect(result.current.tasks).toHaveLength(1); | |
| expect(result.current.tasks[0].description).toBe('Implement feature X'); | |
| expect(result.current.agents[0].currentTaskId).toBe(taskId); |
🧰 Tools
🪛 GitHub Actions: CI
[error] 175-175: lint/style/noNonNullAssertion: Forbidden non-null assertion. (taskId!)
🤖 Prompt for AI Agents
In `@src/stores/__tests__/agentStore.test.ts` around lines 164 - 176, The test
uses a non-null assertion on taskId; to satisfy the noNonNullAssertion lint
rule, initialize taskId instead of using "taskId!" or change its type—e.g.,
replace "let taskId: string;" with "let taskId = '';" (or "let taskId: string |
undefined = undefined;" and adjust the expect to check definedness) so you can
remove the non-null assertion when comparing
result.current.agents[0].currentTaskId to taskId; update references to taskId
and keep the act(() => { taskId = result.current.addTask(...) }) and subsequent
expects unchanged except removing the "!".
Comment on lines
+193
to
+204
| let taskId: string; | ||
| act(() => { | ||
| taskId = result.current.addTask({ | ||
| agentId: 'test-agent-1', | ||
| description: 'Implement feature X', | ||
| status: 'in_progress', | ||
| }); | ||
| }); | ||
|
|
||
| act(() => { | ||
| result.current.completeTask(taskId!, 'Feature implemented successfully'); | ||
| }); |
Contributor
There was a problem hiding this comment.
Fix non-null assertion to resolve lint failure.
Same issue as above - initialize taskId to avoid the non-null assertion.
🔧 Suggested fix
- let taskId: string;
+ let taskId = '';
act(() => {
taskId = result.current.addTask({
agentId: 'test-agent-1',
description: 'Implement feature X',
status: 'in_progress',
});
});
act(() => {
- result.current.completeTask(taskId!, 'Feature implemented successfully');
+ result.current.completeTask(taskId, 'Feature implemented successfully');
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let taskId: string; | |
| act(() => { | |
| taskId = result.current.addTask({ | |
| agentId: 'test-agent-1', | |
| description: 'Implement feature X', | |
| status: 'in_progress', | |
| }); | |
| }); | |
| act(() => { | |
| result.current.completeTask(taskId!, 'Feature implemented successfully'); | |
| }); | |
| let taskId = ''; | |
| act(() => { | |
| taskId = result.current.addTask({ | |
| agentId: 'test-agent-1', | |
| description: 'Implement feature X', | |
| status: 'in_progress', | |
| }); | |
| }); | |
| act(() => { | |
| result.current.completeTask(taskId, 'Feature implemented successfully'); | |
| }); |
🧰 Tools
🪛 GitHub Actions: CI
[error] 203-203: lint/style/noNonNullAssertion: Forbidden non-null assertion. (taskId!)
🤖 Prompt for AI Agents
In `@src/stores/__tests__/agentStore.test.ts` around lines 193 - 204, The test
uses a non-null assertion on taskId; initialize taskId when declaring it to
avoid the assertion and satisfy the linter. Change the declaration of taskId
(used with result.current.addTask and result.current.completeTask) to a defined
default (e.g., let taskId = ''; or assign immediately from addTask) and then
call completeTask(taskId, '...') without the trailing ! so the calls to addTask
and completeTask (result.current.addTask / result.current.completeTask) no
longer require a non-null assertion.
Comment on lines
+243
to
+246
| clearThread: (threadId) => | ||
| set((state) => { | ||
| state.messages[threadId] = []; | ||
| }), |
Contributor
There was a problem hiding this comment.
Reset unread count when clearing a thread’s messages.
clearThread drops messages but leaves unreadCount unchanged, which can show badges for an empty thread.
💡 Suggested fix
clearThread: (threadId) =>
set((state) => {
state.messages[threadId] = [];
+ const thread = state.threads.find((t) => t.id === threadId);
+ if (thread) {
+ thread.unreadCount = 0;
+ }
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| clearThread: (threadId) => | |
| set((state) => { | |
| state.messages[threadId] = []; | |
| }), | |
| clearThread: (threadId) => | |
| set((state) => { | |
| state.messages[threadId] = []; | |
| const thread = state.threads.find((t) => t.id === threadId); | |
| if (thread) { | |
| thread.unreadCount = 0; | |
| } | |
| }), |
🤖 Prompt for AI Agents
In `@src/stores/chatStore.ts` around lines 243 - 246, clearThread currently
empties state.messages[threadId] but doesn't touch the unread count, leaving
badges for an empty thread; update the clearThread handler (the clearThread
function that calls set and mutates state.messages[threadId]) to also reset the
unread count for that thread by setting state.unreadCount[threadId] = 0 (or
state.unreadCounts[threadId] = 0 if your store uses plural naming), and guard
with a null/undefined check (e.g., ensure state.unreadCount exists) so clearing
a thread always clears its unread counter.
Comment on lines
+177
to
+182
| invalidateAll: () => | ||
| set((state) => { | ||
| state.credentials.forEach((credential) => { | ||
| credential.status = 'unknown'; | ||
| }); | ||
| }), |
Contributor
There was a problem hiding this comment.
Clear validation metadata when invalidating credentials.
After invalidateAll, keeping lastValidatedAt/expiresAt can leave stale validation context. Clearing those fields avoids misleading UI/logic.
♻️ Suggested fix
invalidateAll: () =>
set((state) => {
state.credentials.forEach((credential) => {
credential.status = 'unknown';
+ credential.lastValidatedAt = undefined;
+ credential.expiresAt = undefined;
});
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| invalidateAll: () => | |
| set((state) => { | |
| state.credentials.forEach((credential) => { | |
| credential.status = 'unknown'; | |
| }); | |
| }), | |
| invalidateAll: () => | |
| set((state) => { | |
| state.credentials.forEach((credential) => { | |
| credential.status = 'unknown'; | |
| credential.lastValidatedAt = undefined; | |
| credential.expiresAt = undefined; | |
| }); | |
| }), |
🤖 Prompt for AI Agents
In `@src/stores/credentialStore.ts` around lines 177 - 182, The invalidateAll
setter currently only flips credential.status to 'unknown' but leaves validation
metadata, so update the invalidateAll implementation (the set(...) callback that
iterates state.credentials) to also clear the validation fields by setting each
credential.lastValidatedAt and credential.expiresAt to null (or undefined per
your codebase convention) when you set status = 'unknown'; reference the
invalidateAll method, state.credentials array, and the lastValidatedAt/expiresAt
properties to locate and update the logic.
Comment on lines
+127
to
+135
| removeProject: (projectId) => | ||
| set((state) => { | ||
| state.projects = state.projects.filter((p) => p.id !== projectId); | ||
| if (state.activeProjectId === projectId) { | ||
| state.activeProjectId = null; | ||
| state.workspace = null; | ||
| state.fileTree = null; | ||
| } | ||
| }), |
Contributor
There was a problem hiding this comment.
Reset branch/commit state when removing the active project.
If the active project is removed, branches and recentCommits retain stale data. That can surface incorrect Git info after removal.
🐛 Suggested fix
removeProject: (projectId) =>
set((state) => {
state.projects = state.projects.filter((p) => p.id !== projectId);
if (state.activeProjectId === projectId) {
state.activeProjectId = null;
state.workspace = null;
state.fileTree = null;
+ state.branches = [];
+ state.recentCommits = [];
}
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| removeProject: (projectId) => | |
| set((state) => { | |
| state.projects = state.projects.filter((p) => p.id !== projectId); | |
| if (state.activeProjectId === projectId) { | |
| state.activeProjectId = null; | |
| state.workspace = null; | |
| state.fileTree = null; | |
| } | |
| }), | |
| removeProject: (projectId) => | |
| set((state) => { | |
| state.projects = state.projects.filter((p) => p.id !== projectId); | |
| if (state.activeProjectId === projectId) { | |
| state.activeProjectId = null; | |
| state.workspace = null; | |
| state.fileTree = null; | |
| state.branches = []; | |
| state.recentCommits = []; | |
| } | |
| }), |
🤖 Prompt for AI Agents
In `@src/stores/projectStore.ts` around lines 127 - 135, When removing the active
project in removeProject, branches and recentCommits are left populated causing
stale Git info; update the removeProject handler to also clear state.branches
and state.recentCommits when state.activeProjectId === projectId (similar to how
state.workspace and state.fileTree are set to null) so that state.branches = []
and state.recentCommits = [] (or null if your store uses null) and thereby fully
reset branch/commit state for the removed active project.
22 tasks
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
Implements comprehensive Zustand-based state management as specified in Issue #10.
Stores Added:
Features:
Architecture Highlights:
Test plan
Related Issues
Closes #10
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.