Skip to content

Commit 13bb1b0

Browse files
Fix repo reset and stale base-branch in MCP App views
- Re-initialize selectedRepo from toolInput inside the reset-on-invocation effect instead of a separate effect. The two effects both depended on toolInput and ran in declaration order, so the reset wiped the just- initialized repo and the picker never reflected the invocation's owner/repo. - Set the default base branch with a functional update in pr-write so a base prefilled from toolInput.base (or chosen by the user) isn't overwritten by a stale baseBranch value captured before the branches request resolved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d9292e9 commit 13bb1b0

2 files changed

Lines changed: 32 additions & 32 deletions

File tree

ui/src/apps/issue-write/App.tsx

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -229,19 +229,6 @@ function CreateIssueApp() {
229229
const owner = selectedRepo?.owner || (toolInput?.owner as string) || "";
230230
const repo = selectedRepo?.name || (toolInput?.repo as string) || "";
231231

232-
// Initialize selectedRepo from toolInput
233-
useEffect(() => {
234-
if (toolInput?.owner && toolInput?.repo && !selectedRepo) {
235-
setSelectedRepo({
236-
id: `${toolInput.owner}/${toolInput.repo}`,
237-
owner: toolInput.owner as string,
238-
name: toolInput.repo as string,
239-
fullName: `${toolInput.owner}/${toolInput.repo}`,
240-
isPrivate: false,
241-
});
242-
}
243-
}, [toolInput, selectedRepo]);
244-
245232
// Search repositories when filter changes
246233
useEffect(() => {
247234
if (!app || !repoFilter.trim()) {
@@ -429,7 +416,8 @@ function CreateIssueApp() {
429416
// Reset all transient form/result state when toolInput changes (new invocation).
430417
// Without this, the SuccessView from a previous submit stays visible and stale
431418
// form values (e.g. body) bleed through because prefill effects use truthy guards
432-
// that won't overwrite with empty values.
419+
// that won't overwrite with empty values. The repo is re-initialized from the new
420+
// invocation here (rather than in a separate effect) so it isn't wiped by this reset.
433421
useEffect(() => {
434422
prefillApplied.current = { title: false, body: false, labels: false, assignees: false, milestone: false, type: false };
435423
setExistingIssueData(null);
@@ -441,7 +429,17 @@ function CreateIssueApp() {
441429
setSelectedIssueType(null);
442430
setSuccessIssue(null);
443431
setError(null);
444-
setSelectedRepo(null);
432+
if (toolInput?.owner && toolInput?.repo) {
433+
setSelectedRepo({
434+
id: `${toolInput.owner}/${toolInput.repo}`,
435+
owner: toolInput.owner as string,
436+
name: toolInput.repo as string,
437+
fullName: `${toolInput.owner}/${toolInput.repo}`,
438+
isPrivate: false,
439+
});
440+
} else {
441+
setSelectedRepo(null);
442+
}
445443
}, [toolInput]);
446444

447445
// Load existing issue data when in update mode

ui/src/apps/pr-write/App.tsx

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -165,23 +165,12 @@ function CreatePRApp() {
165165
const repo = selectedRepo?.name || (toolInput?.repo as string) || "";
166166
const [submittedTitle, setSubmittedTitle] = useState("");
167167

168-
// Initialize selectedRepo from toolInput
169-
useEffect(() => {
170-
if (toolInput?.owner && toolInput?.repo && !selectedRepo) {
171-
setSelectedRepo({
172-
id: `${toolInput.owner}/${toolInput.repo}`,
173-
owner: toolInput.owner as string,
174-
name: toolInput.repo as string,
175-
fullName: `${toolInput.owner}/${toolInput.repo}`,
176-
isPrivate: false,
177-
});
178-
}
179-
}, [toolInput, selectedRepo]);
180-
181168
// Reset all transient form/result state when toolInput changes (new invocation).
182169
// Without this, the SuccessView from a previous submit stays visible and stale
183170
// form values bleed through because the prefill effect below only sets when
184-
// toolInput has truthy values and never clears.
171+
// toolInput has truthy values and never clears. The repo is re-initialized from
172+
// the new invocation here (rather than in a separate effect) so it isn't wiped
173+
// by this reset.
185174
useEffect(() => {
186175
setTitle("");
187176
setBody("");
@@ -192,7 +181,17 @@ function CreatePRApp() {
192181
setSuccessPR(null);
193182
setError(null);
194183
setSubmittedTitle("");
195-
setSelectedRepo(null);
184+
if (toolInput?.owner && toolInput?.repo) {
185+
setSelectedRepo({
186+
id: `${toolInput.owner}/${toolInput.repo}`,
187+
owner: toolInput.owner as string,
188+
name: toolInput.repo as string,
189+
fullName: `${toolInput.owner}/${toolInput.repo}`,
190+
isPrivate: false,
191+
});
192+
} else {
193+
setSelectedRepo(null);
194+
}
196195
}, [toolInput]);
197196

198197
// Pre-fill from toolInput
@@ -261,9 +260,12 @@ function CreatePRApp() {
261260
(b: { name: string; protected?: boolean }) => ({ name: b.name, protected: b.protected || false })
262261
);
263262
setAvailableBranches(branches);
264-
if (!baseBranch && branches.length > 0) {
263+
if (branches.length > 0) {
265264
const defaultBranch = branches.find((b: BranchItem) => b.name === 'main' || b.name === 'master');
266-
if (defaultBranch) setBaseBranch(defaultBranch.name);
265+
// Functional update so a base branch already prefilled from
266+
// toolInput.base (or chosen by the user) isn't overwritten by a
267+
// stale closure value captured before the request resolved.
268+
if (defaultBranch) setBaseBranch((prev) => prev || defaultBranch.name);
267269
}
268270
}
269271
}

0 commit comments

Comments
 (0)