[bug/5] Resolved push hanging when push fails#6
Merged
justinkumpe merged 1 commit intomainfrom Feb 13, 2026
Merged
Conversation
Reviewer's GuideRefactors the Git QuickOps webview to use a safer, programmatic DOM-building approach and a unified VS Code API bridge, while making git push behavior robust by resolving upstream/remote interactively, reusing a shared remote-add flow, and improving activation and loading behavior across views. Sequence diagram for robust git push with remote and upstream resolutionsequenceDiagram
actor User
participant VSCode
participant ExtensionCommands
participant GitModule
participant VSCodeUI
User->>VSCode: Run cmdPush or cmdAddCommitPush
VSCode->>ExtensionCommands: cmdPush / cmdAddCommitPush
ExtensionCommands->>RepositoryContext: getGitRoot
RepositoryContext-->>ExtensionCommands: gitRoot
ExtensionCommands->>GitModule: getCurrentBranch gitRoot
GitModule-->>ExtensionCommands: branch
ExtensionCommands->>ExtensionCommands: resolvePushTarget gitRoot branch
alt branch has upstream
ExtensionCommands->>GitModule: getBranchUpstream gitRoot branch
GitModule-->>ExtensionCommands: remote and upstreamBranch
ExtensionCommands-->>ExtensionCommands: pushTarget args push and display remote upstreamBranch
else no upstream
ExtensionCommands->>VSCodeUI: read config gitQuickOps.defaultRemote
ExtensionCommands->>GitModule: getRemotes gitRoot
GitModule-->>ExtensionCommands: remotes
alt no remotes
ExtensionCommands->>VSCodeUI: showWarningMessage Add Remote or Cancel
VSCodeUI-->>ExtensionCommands: user choice
alt user cancels
ExtensionCommands-->>ExtensionCommands: resolvePushTarget returns null
ExtensionCommands-->>VSCode: return without pushing
else user chooses Add Remote
ExtensionCommands->>VSCodeUI: promptAddRemote name and url
loop validate name and url
VSCodeUI-->>ExtensionCommands: user input
ExtensionCommands->>VSCodeUI: showErrorMessage on invalid input
end
alt remote exists
ExtensionCommands->>GitModule: execGit remote set url
else new remote
ExtensionCommands->>GitModule: execGit remote add
end
ExtensionCommands->>VSCodeUI: showInformationMessage Remote configured
ExtensionCommands->>GitModule: getRemotes gitRoot
GitModule-->>ExtensionCommands: remotes updated
end
end
alt defaultRemote in remotes
ExtensionCommands-->>ExtensionCommands: selectedRemote is defaultRemote
else multiple remotes
ExtensionCommands->>VSCodeUI: quickPick remotes plus Add new remote
VSCodeUI-->>ExtensionCommands: selected item
alt user selects Add new remote
ExtensionCommands->>VSCodeUI: promptAddRemote
VSCodeUI-->>ExtensionCommands: remoteName or cancellation
alt cancelled
ExtensionCommands-->>ExtensionCommands: resolvePushTarget returns null
ExtensionCommands-->>VSCode: return without pushing
else remote created
ExtensionCommands-->>ExtensionCommands: selectedRemote is new remote
end
else user selects existing remote
ExtensionCommands-->>ExtensionCommands: selectedRemote is chosen label
end
end
ExtensionCommands->>VSCodeUI: quickPick Yes or No to set upstream
VSCodeUI-->>ExtensionCommands: user choice
alt user declines
ExtensionCommands-->>ExtensionCommands: resolvePushTarget returns null
ExtensionCommands-->>VSCode: return without pushing
else user accepts
ExtensionCommands-->>ExtensionCommands: pushTarget args push u remote branch and display
end
end
alt pushTarget is not null
ExtensionCommands->>VSCodeUI: withProgress Pushing to display
VSCodeUI->>GitModule: execGit or runGitCommand gitRoot pushTarget args
alt git push succeeds
GitModule-->>VSCodeUI: success
VSCodeUI-->>ExtensionCommands: progress complete
ExtensionCommands->>VSCodeUI: showInformationMessage push success
else git push fails
GitModule-->>VSCodeUI: error
VSCodeUI-->>ExtensionCommands: propagate error
ExtensionCommands->>VSCodeUI: showErrorMessage Push failed error
end
end
Sequence diagram for unified webview command posting from UI elementssequenceDiagram
participant User
participant WebviewDOM
participant WebviewScript
participant VSCodeApi
participant WebviewProvider
participant ExtensionLogic
User->>WebviewDOM: Click UI element for example Stage file
WebviewDOM->>WebviewScript: Event listener callback
WebviewScript->>WebviewScript: Determine command stageFile and path
WebviewScript->>VSCodeApi: postMessage command stageFile with path
VSCodeApi->>WebviewProvider: onDidReceiveMessage
WebviewProvider->>WebviewProvider: _handleMessage switch on data command
WebviewProvider->>ExtensionLogic: stage file in repository
User->>WebviewDOM: Click navigation item
WebviewDOM->>WebviewScript: Event handler for navigateTo or navigateToView
WebviewScript->>VSCodeApi: postMessage command navigateTo or navigateToView
VSCodeApi->>WebviewProvider: onDidReceiveMessage
WebviewProvider->>ExtensionLogic: perform navigation and refresh view
User->>WebviewDOM: Submit setup form
WebviewDOM->>WebviewScript: saveSetup handler reads commitPrefix and requireTests
WebviewScript->>VSCodeApi: postMessage command saveSetup with config
VSCodeApi->>WebviewProvider: onDidReceiveMessage
WebviewProvider->>ExtensionLogic: persist configuration and update views
Flow diagram for webview renderView and view specific renderingflowchart TD
A_start[Start renderView data]
A_check_data{data exists}
A_error_no_data[Render error No data received from extension]
A_check_error{data error field}
A_render_error[renderError data error]
A_switch_type{data type}
B_repos[renderRepositories]
C_menu[renderNavigation]
D_setup[renderSetup]
E_changes[renderChanges]
F_commits[renderCommits]
G_unknown[renderError Unknown view data type]
H_catch_error[Catch exception and log]
H_render_error[Render error Failed to render view]
A_start --> A_check_data
A_check_data -- no --> A_error_no_data --> H_end[End]
A_check_data -- yes --> A_check_error
A_check_error -- yes --> A_render_error --> H_end
A_check_error -- no --> A_switch_type
A_switch_type -- repositories --> B_repos --> H_end
A_switch_type -- menu --> C_menu --> H_end
A_switch_type -- setup --> D_setup --> H_end
A_switch_type -- changes --> E_changes --> H_end
A_switch_type -- commits --> F_commits --> H_end
A_switch_type -- other --> G_unknown --> H_end
A_start --> H_try[Try block]
H_try --> A_check_data
H_try --> H_catch_error
H_catch_error --> H_render_error --> H_end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
selectRemoteForPush, the QuickPick items are custom objects with avaluefield; consider using propervscode.QuickPickItemtyping (e.g. storing the remote name inlabeland checkingselected.label) to avoid TypeScript type mismatches and make the API usage clearer. - In
renderCommitItem,const author = commit.author.split(' ')[0];will truncate multi-word names and behave oddly with leading spaces; consider using a more robust approach (e.g. using the full author string or splitting on<if you want the name portion).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `selectRemoteForPush`, the QuickPick items are custom objects with a `value` field; consider using proper `vscode.QuickPickItem` typing (e.g. storing the remote name in `label` and checking `selected.label`) to avoid TypeScript type mismatches and make the API usage clearer.
- In `renderCommitItem`, `const author = commit.author.split(' ')[0];` will truncate multi-word names and behave oddly with leading spaces; consider using a more robust approach (e.g. using the full author string or splitting on `<` if you want the name portion).
## Individual Comments
### Comment 1
<location> `src/extension.ts:858-860` </location>
<code_context>
+ return selected.value;
+}
+
+async function promptAddRemote(gitRoot: string): Promise<string | null> {
+ const existingRemotes = await git.getRemotes(gitRoot);
+ const remoteName = await vscode.window.showInputBox({
+ prompt: 'Enter remote name',
+ placeHolder: 'origin'
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validate `remoteName` and `remoteUrl` inputs to avoid obviously invalid values.
Right now any non-empty string is accepted for `remoteName`/`remoteUrl`, so values with leading/trailing spaces or spaces in the name can slip through and later break `git remote` commands.
Please trim both inputs and add minimal validation, e.g. reject names containing whitespace and empty URLs, and either show an error or re-prompt the user.
Suggested implementation:
```typescript
const existingRemotes = await git.getRemotes(gitRoot);
let remoteName: string;
while (true) {
const input = await vscode.window.showInputBox({
prompt: 'Enter remote name',
placeHolder: 'origin'
});
if (!input) {
// User cancelled the input
return null;
}
const trimmed = input.trim();
if (!trimmed) {
vscode.window.showErrorMessage('Remote name cannot be empty or whitespace.');
continue;
}
if (/\s/.test(trimmed)) {
vscode.window.showErrorMessage('Remote name cannot contain whitespace.');
continue;
}
remoteName = trimmed;
break;
}
let remoteUrl: string;
while (true) {
const input = await vscode.window.showInputBox({
prompt: 'Enter remote URL',
placeHolder: 'https://github.com/user/repo.git'
});
if (!input) {
// User cancelled the input
return null;
}
const trimmed = input.trim();
if (!trimmed) {
vscode.window.showErrorMessage('Remote URL cannot be empty or whitespace.');
continue;
}
remoteUrl = trimmed;
break;
}
```
1. Ensure that any subsequent usage of `remoteName` and `remoteUrl` within `promptAddRemote` relies on these now-trimmed variables (the identifiers are unchanged, so this should be transparent).
2. If you want to prevent duplicate remote names using `existingRemotes`, you could extend the `remoteName` validation loop to check `existingRemotes.some(r => r.name === trimmed)` and show an error if it already exists.
</issue_to_address>
### Comment 2
<location> `media/main.js:671-674` </location>
<code_context>
+ }
- window.saveSetup = function() {
+ function saveSetup() {
+ const commitPrefix = document.getElementById('commitPrefix');
+ const requireTests = document.getElementById('requireTests');
const config = {
- commitPrefix: document.getElementById('commitPrefix').value,
- requireTests: document.getElementById('requireTests').value
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid silently falling back to default config values when `commitPrefix` or `requireTests` elements are missing.
Right now, if either element lookup returns `null`, you end up posting a config with `''` / `'disabled'`, overwriting the existing config with defaults without any indication to the user.
Add an explicit guard so you fail fast instead of silently resetting:
```js
if (!commitPrefix || !requireTests) {
console.error('Setup form controls not found');
return;
}
```
This makes DOM issues visible and prevents unintended config changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai resolve |
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- By removing the
onCommand:git-quickops.showMenuactivation event and only activating on views, invoking thegit-quickops.showMenucommand from the command palette may no longer activate the extension; consider keeping the command-based activation if the command is meant to be callable independently of the views. - The push error handlers currently display
Push failed: ${error}, which may stringify as[object Object]; consider normalizing the error (e.g., checkingerror instanceof Error ? error.message : String(error)) for clearer user-facing messages. - The push flow in
cmdAddCommitPushandcmdPushnow shares the sameresolvePushTargetlogic but still duplicates the progress and notification handling; consider extracting a common helper to run the push and show messages so behavior stays consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- By removing the `onCommand:git-quickops.showMenu` activation event and only activating on views, invoking the `git-quickops.showMenu` command from the command palette may no longer activate the extension; consider keeping the command-based activation if the command is meant to be callable independently of the views.
- The push error handlers currently display `Push failed: ${error}`, which may stringify as `[object Object]`; consider normalizing the error (e.g., checking `error instanceof Error ? error.message : String(error)`) for clearer user-facing messages.
- The push flow in `cmdAddCommitPush` and `cmdPush` now shares the same `resolvePushTarget` logic but still duplicates the progress and notification handling; consider extracting a common helper to run the push and show messages so behavior stays consistent and easier to maintain.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
resolves #5
Summary by Sourcery
Improve webview initialization and DOM-based rendering while making git push behavior more robust and interactive.
New Features:
Bug Fixes:
Enhancements: