mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-10 01:29:17 +00:00
fix(worktree): address PR #4174 round 4 findings
Finding #3256237933 (Critical, follow-up to #3252368640 part 1): handleWorktreeExit silently /quit'd when removeUserWorktree returned {success:false}, contradicting the user's intent after they clicked "Remove worktree and branch (discards N commits, M files)". Now surfaces an ERROR history item with the underlying error message and STAYS in the session so the user can decide what to do (retry via exit_worktree, fix the lock/permission/corruption issue, or quit anyway). Same treatment applied to the hard-failure catch block — previously it caught the throw and proceeded to /quit with no log; now it emits the error and stays alive. Finding #3256236050 (Nit): originalCwd field name implies "user's launch cwd" but actually stores `getRepoTopLevel()` (different in monorepo subdir launches — the gap closed by #3252368637). Renaming the field would force on-disk migration of every existing sidecar (every active --resume breaks until users wipe the old file). Doc-only fix: WorktreeSession.originalCwd now carries an explicit JSDoc explaining the semantics and warning consumers expecting process.cwd() to NOT use this field. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
ce8ed08415
commit
d4e921ae29
2 changed files with 51 additions and 8 deletions
|
|
@ -1164,10 +1164,25 @@ export const AppContainer = (props: AppContainerProps) => {
|
|||
// removeUserWorktree returns {success, error} on failure — it
|
||||
// does NOT throw — so the previous try/catch never tripped on
|
||||
// a soft failure. If removal failed, leave the sidecar intact
|
||||
// so the next --resume can still see the worktree and let the
|
||||
// user retry. (Finding 3252368640 part 1.)
|
||||
// so the next --resume can still see the worktree. Surface
|
||||
// the error in history and stay in the session so the user
|
||||
// can decide what to do (retry via exit_worktree, fix the
|
||||
// underlying problem, or force-quit). Previously the dialog
|
||||
// silently /quit on failure, contradicting the "discards N
|
||||
// commits, M files" intent the user clicked Remove on.
|
||||
// (Findings 3252368640 part 1 + 3256237933.)
|
||||
if (!result.success) {
|
||||
handleSlashCommand('/quit');
|
||||
historyManager.addItem(
|
||||
{
|
||||
type: MessageType.ERROR,
|
||||
text:
|
||||
`Failed to remove worktree "${activeWorktree.slug}": ` +
|
||||
`${result.error ?? 'unknown error'}. The worktree is ` +
|
||||
`still on disk; use \`exit_worktree\` to retry or ` +
|
||||
`remove it manually with \`git worktree remove\`.`,
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
return;
|
||||
}
|
||||
await clearWorktreeSession(
|
||||
|
|
@ -1175,15 +1190,28 @@ export const AppContainer = (props: AppContainerProps) => {
|
|||
.getSessionService()
|
||||
.getWorktreeSessionPath(config.getSessionId()),
|
||||
);
|
||||
} catch {
|
||||
// Hard failure (e.g. git binary missing) — proceed with quit
|
||||
// anyway so the user isn't stranded. Sidecar stays so
|
||||
// --resume can recover.
|
||||
} catch (error) {
|
||||
// Hard failure (e.g. git binary missing, GitWorktreeService
|
||||
// constructor threw). Same treatment as the soft failure
|
||||
// path: surface to the user and stay alive — silent /quit
|
||||
// here would leave the user wondering whether the worktree
|
||||
// was actually removed.
|
||||
historyManager.addItem(
|
||||
{
|
||||
type: MessageType.ERROR,
|
||||
text:
|
||||
`Worktree removal failed for "${activeWorktree.slug}": ` +
|
||||
`${error instanceof Error ? error.message : String(error)}. ` +
|
||||
`Use \`exit_worktree\` or remove it manually.`,
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
handleSlashCommand('/quit');
|
||||
},
|
||||
[activeWorktree, config, handleSlashCommand],
|
||||
[activeWorktree, config, handleSlashCommand, historyManager],
|
||||
);
|
||||
|
||||
const performMemoryRefresh = useCallback(async () => {
|
||||
|
|
|
|||
|
|
@ -21,6 +21,21 @@ export interface WorktreeSession {
|
|||
slug: string;
|
||||
worktreePath: string;
|
||||
worktreeBranch: string;
|
||||
/**
|
||||
* The repo top-level (output of `GitWorktreeService.getRepoTopLevel()`)
|
||||
* captured when the worktree was created — NOT the user's launch cwd.
|
||||
*
|
||||
* Named `originalCwd` for on-disk back-compat with sidecars written
|
||||
* by earlier Phase C builds; semantically this is the value to pass
|
||||
* back to `new GitWorktreeService(...)` for any subsequent cleanup
|
||||
* (e.g. `handleWorktreeExit`'s remove path), because the worktree
|
||||
* always lives under `<repoTopLevel>/.qwen/worktrees/`. When the
|
||||
* CLI is launched from a monorepo subdirectory, `process.cwd()` and
|
||||
* `getRepoTopLevel()` differ — this field stores the latter.
|
||||
*
|
||||
* Consumers expecting `process.cwd()` semantics should NOT use this
|
||||
* field; capture cwd separately at the time of need.
|
||||
*/
|
||||
originalCwd: string;
|
||||
originalBranch: string;
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue