fix: duplicate agent name reset and missing group inheritance#833
fix: duplicate agent name reset and missing group inheritance#833pedramamini wants to merge 3 commits intomainfrom
Conversation
Fix two bugs in the duplicate agent flow: 1. Name reverts to "(Copy)" on save: The useEffect depended on the full sourceSession object, which gets a new reference on any sessions store update, re-triggering the pre-fill and overwriting the user's typed name. Changed dependency to sourceSession?.id so it only fires when the identity changes. 2. Group not inherited: sourceSession.groupId was never threaded through the duplication flow. Added groupId parameter to onCreate, onCreateSession, and createNewSession, passing sourceSession?.groupId from handleCreate. Closes #827
📝 WalkthroughWalkthroughThis PR fixes two bugs in the agent duplication flow: (1) corrects a modal effect dependency to prevent form values from reverting when the sessions store updates, and (2) threads Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR fixes two bugs in the duplicate-agent flow: a Confidence Score: 5/5Safe to merge — both bugs are correctly fixed and all findings are P2 (style/coverage) that don't block the changes. Both root causes are correctly addressed: the useEffect dependency is narrowed to sourceSession?.id and groupId is properly threaded through the full call chain. The only findings are a missing useCallback dependency (stale closure that can't produce a visible bug today) and a missing test case for groupId forwarding. All 72 modal tests pass. src/renderer/components/NewInstanceModal.tsx — handleCreate deps array is missing sourceSession. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant NIM as NewInstanceModal
participant AM as AppModals
participant CRUD as useSessionCrud
participant Store as sessionStore
U->>AM: click "Duplicate Agent"
AM->>AM: useMemo sourceSession from sessions.find(id)
AM->>NIM: sourceSession prop
NIM->>NIM: sourceSessionId = sourceSession?.id
NIM->>NIM: useEffect([isOpen, sourceSessionId]): loadAgents(sourceSession)
Note over NIM: Pre-fills name as "{name} (Copy)",<br/>workingDir, agentType from sourceSession
Store-->>AM: sessions update (new reference)
AM->>AM: useMemo re-runs, sourceSession same id
AM->>NIM: new sourceSession reference, same id
Note over NIM: useEffect does NOT re-fire<br/>(sourceSessionId unchanged)<br/>User's typed name is preserved
U->>NIM: click "Create Agent"
NIM->>NIM: handleCreate() reads sourceSession?.groupId
NIM->>AM: onCreate(agentId, dir, name, ..., groupId)
AM->>CRUD: onCreateSession(agentId, dir, name, ..., groupId)
CRUD->>Store: setSessions([...prev, newSession{groupId}])
Note over Store: Duplicated agent lands in same group
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/NewInstanceModal.tsx (1)
471-484:⚠️ Potential issue | 🟡 Minor
handleCreatecan capture a stalegroupId.Line 483 reads
sourceSession?.groupId, butsourceSessionis not in theuseCallbackdependency list (lines 500–515). SincesourceSessionis a prop that may change while the modal is open, the callback will use a stale group ID if the source session updates.💡 Suggested fix
+ const sourceGroupId = sourceSession?.groupId; + const handleCreate = React.useCallback(() => { ... onCreate( selectedAgent, expandedWorkingDir, name, nudgeMessage.trim() || undefined, agentCustomPath, agentCustomArgs, agentCustomEnvVars, agentCustomModel, agentCustomContextWindow, agentCustomProviderPath, sessionSshRemoteConfig, - sourceSession?.groupId + sourceGroupId ); ... }, [ instanceName, selectedAgent, workingDir, nudgeMessage, customAgentPaths, customAgentArgs, customAgentEnvVars, agentConfigs, agentSshRemoteConfigs, onCreate, onClose, expandTilde, handleWorkingDirChange, existingSessions, + sourceGroupId, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/NewInstanceModal.tsx` around lines 471 - 484, handleCreate captures a stale sourceSession.groupId because sourceSession isn't listed in the useCallback dependencies; update the useCallback that defines handleCreate to include sourceSession (or at minimum sourceSession?.groupId) in its dependency array so onCreate(...) is invoked with the current groupId, ensuring handleCreate uses the latest sourceSession when calling onCreate.
🧹 Nitpick comments (1)
docs/releases.md (1)
20-20: Use nested heading levels within release sectionsThese headings are currently
#(H1) inside sectioned content. Please switch them to###(or####where appropriate) to preserve Markdown hierarchy and avoid TOC/anchor structure regressions.Also applies to: 80-80, 143-143, 147-147, 151-151, 176-176, 268-268, 276-276, 286-286, 291-291, 297-297
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/releases.md` at line 20, Several release-section headings (e.g., the "Major 0.15.x Additions" heading and the other release headings referenced) are using top-level H1 (#) inside the releases document; change those headings to nested levels (use ### or #### as appropriate) to preserve markdown hierarchy and TOC anchors—scan the releases.md for headings that are currently single '#' within release sections and replace them with the correct subheading level (prefer ### for section titles, use #### for sub-subsections) so the document structure is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/releases.md`:
- Line 74: Fix two visible typos in the release notes: replace the incorrect
capitalized token "FIle" in the sentence starting with "🗄️ Document Graphs.
Launch from file preview..." with "File", and correct the version string "014.x"
to "0.14.x" wherever it appears (notably the occurrence around line 80). Ensure
both replacements preserve surrounding punctuation and capitalization in the
release note text.
- Line 22: Standardize wording in the release notes: replace any occurrence of
"Github" with "GitHub", pick one consistent form for "pre-release"/"prerelease"
and apply it everywhere, and hyphenate compound modifiers such as "open-source",
"cross-context", and "built-in" (e.g., in the "Maestro Symphony" blurb and other
instances). Search for the literal strings "Github", "pre-release"/"prerelease",
"open source"/"open-source", "cross context"/"cross-context", and "built
in"/"built-in" and update them to the chosen, consistent spellings across the
document.
---
Outside diff comments:
In `@src/renderer/components/NewInstanceModal.tsx`:
- Around line 471-484: handleCreate captures a stale sourceSession.groupId
because sourceSession isn't listed in the useCallback dependencies; update the
useCallback that defines handleCreate to include sourceSession (or at minimum
sourceSession?.groupId) in its dependency array so onCreate(...) is invoked with
the current groupId, ensuring handleCreate uses the latest sourceSession when
calling onCreate.
---
Nitpick comments:
In `@docs/releases.md`:
- Line 20: Several release-section headings (e.g., the "Major 0.15.x Additions"
heading and the other release headings referenced) are using top-level H1 (#)
inside the releases document; change those headings to nested levels (use ### or
#### as appropriate) to preserve markdown hierarchy and TOC anchors—scan the
releases.md for headings that are currently single '#' within release sections
and replace them with the correct subheading level (prefer ### for section
titles, use #### for sub-subsections) so the document structure is consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2ab28d3-a934-49e7-b954-406e163ca8f7
📒 Files selected for processing (5)
docs/releases.mdsrc/__tests__/renderer/components/NewInstanceModal.test.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/NewInstanceModal.tsxsrc/renderer/hooks/session/useSessionCrud.ts
| - **Default worktree directory:** Worktree configuration now defaults to the parent of the agent's working directory instead of blank | ||
| # Major 0.15.x Additions | ||
|
|
||
| 🎶 **Maestro Symphony** — Contribute to open source with AI assistance! Browse curated issues from projects with the `runmaestro.ai` label, clone repos with one click, and automatically process the relevant Auto Run playbooks. Track your contributions, streaks, and stats. You're contributing CPU and tokens towards your favorite open source projects and features. |
There was a problem hiding this comment.
Standardize wording and product spelling for consistency
Please normalize a few terms for consistency/readability:
- Line 174:
Github→GitHub - Line 239: choose one form and keep it consistent (
pre-releasevsprerelease) - Line 22 / Line 63 / Line 149: hyphenate compound modifiers where needed (
open-source,cross-context,built-in).
Also applies to: 63-63, 149-149, 174-174, 239-239
🧰 Tools
🪛 LanguageTool
[grammar] ~22-~22: Use a hyphen to join words.
Context: ...PU and tokens towards your favorite open source projects and features. 🎬 **Dire...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/releases.md` at line 22, Standardize wording in the release notes:
replace any occurrence of "Github" with "GitHub", pick one consistent form for
"pre-release"/"prerelease" and apply it everywhere, and hyphenate compound
modifiers such as "open-source", "cross-context", and "built-in" (e.g., in the
"Maestro Symphony" blurb and other instances). Search for the literal strings
"Github", "pre-release"/"prerelease", "open source"/"open-source", "cross
context"/"cross-context", and "built in"/"built-in" and update them to the
chosen, consistent spellings across the document.
|
|
||
| The major contributions to 0.14.x remain: | ||
|
|
||
| 🗄️ Document Graphs. Launch from file preview or from the FIle tree panel. Explore relationships between Markdown documents that contain links between documents and to URLs. |
There was a problem hiding this comment.
Fix visible typos in release notes
There are two user-facing typos: Line 74 uses FIle and Line 80 uses 014.x (missing dot after zero). These should be corrected for release-note quality.
Also applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/releases.md` at line 74, Fix two visible typos in the release notes:
replace the incorrect capitalized token "FIle" in the sentence starting with
"🗄️ Document Graphs. Launch from file preview..." with "File", and correct the
version string "014.x" to "0.14.x" wherever it appears (notably the occurrence
around line 80). Ensure both replacements preserve surrounding punctuation and
capitalization in the release note text.
1 participant
Summary
Fixes two bugs in the duplicate agent flow reported in #827:
useEffectinNewInstanceModaldepended on the fullsourceSessionobject. When the sessions store updated during modal interaction,sourceSessiongot a new reference, re-fired the effect, and overwrote the user's typed name with{name} (Copy). Fixed by depending onsourceSession?.idinstead.sourceSession.groupIdwas never passed through the duplication call chain. ThreadedgroupIdfromsourceSessionthroughhandleCreate→onCreate→onCreateSession→createNewSessionso the duplicated agent lands in the same group.Test plan
NewInstanceModaltests pass (updated 7 assertions for newgroupIdparameter)npm run lintpassesnpm run testpasses (22444 tests)Closes #827
Summary by CodeRabbit
Documentation
Tests