Conversation
- Update package description to use lowercase elizaos - Fix minor whitespace formatting in source files
WalkthroughAdds workspace-aware Git utilities, a new gitSettings provider exposing non-sensitive runtime and git status, safer concurrent state loading, and tightened branch-name extraction heuristics across several git-related actions; plus miscellaneous formatting, .gitignore, and build-info removal. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Provider as gitSettingsProvider
participant GitSvc as GitService
participant Runtime
participant Workspace as WorkspacePlugin
participant State as StateStore
Client->>Provider: request GIT_SETTINGS.get(conversationId)
Provider->>Runtime: read plugin config & auth settings
Provider->>GitSvc: isGitInstalled(), getGitVersion()
GitSvc->>Workspace: getActiveWorkspacePath(conversationId)
alt Workspace has active path
Workspace-->>GitSvc: active workspace path
else
Workspace-->>GitSvc: null
end
Provider->>State: loadGitState(conversationId)
State-->>Provider: git state (working copies, activeRepo)
Provider-->>Client: ProviderResult { text, values, data }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
There was a problem hiding this comment.
Pull request overview
This PR performs minor whitespace cleanup across the plugin-git codebase. While the PR description mentions updating the package description to use lowercase "elizaos", the actual changes only remove trailing whitespace and blank lines from source files.
Changes:
- Remove trailing whitespace from filter expression in src/plugin.ts
- Remove trailing whitespace from export statements in src/index.ts
- Remove trailing blank line from package.json
Reviewed changes
Copilot reviewed 1 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/plugin.ts | Removed trailing whitespace from filter function on line 99 |
| src/index.ts | Removed trailing whitespace from two export statement sections |
| package.json | Removed trailing blank line at end of file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
The PR description mentions "Update package description to use lowercase elizaos", but the actual diff for package.json only shows removal of a trailing blank line. There don't appear to be any branding changes in the description field. Either the description field was already using lowercase "elizaos", or the PR description is inaccurate.
- Add isGitInstalled() and getGitVersion() methods to GitService - Add getActiveWorkspacePath() and resolveTargetPath() for plugin-workspace integration - Fix null safety in log.ts and status.ts validate functions - These methods are required by gitInstructionsProvider Resolves "gitService.isGitInstalled is not a function" runtime error Co-authored-by: Cursor <cursoragent@cursor.com>
- Add gitSettingsProvider to expose current configuration (non-sensitive) - Enhance getActiveRepository() to check plugin-workspace active workspace - Add progressive enhancement: explicit repo > workspace > most recent clone - Add getActiveWorkspacePath() and resolveTargetPath() to GitService - Update plugin registration to include new settings provider This enables seamless integration with plugin-workspace while maintaining standalone functionality when workspace plugin is not loaded. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace per-provider cache loads with a shared promise pattern. When multiple providers call loadGitState() during a single request, they now share the same promise instead of each hitting the cache. Before: 10-20 cache loads per message (one per provider call) After: 1 cache load per message (shared across all providers) This eliminates the noisy debug logging and improves performance by avoiding redundant cache hits. Co-authored-by: Cursor <cursoragent@cursor.com>
- checkout/clone/log: skip filler words (the, a, on) so they are not parsed as branch names - checkout: quoted branch names, stop words, reverse 'X branch' pattern - pull/push: require 'branch' keyword before branch name - settings: parseInt(String(cloneTimeout)) for safe parsing - Add .gitignore; stop tracking tsconfig.tsbuildinfo Made-with: Cursor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/push.ts (1)
68-73:⚠️ Potential issue | 🟠 MajorBranch parsing now misreads common phrasing and can target the wrong branch.
At Line 69,
branchextraction only supportsbranch <name>. For prompts like “feature branch to origin” (already used in examples), it capturestoas the branch.💡 Proposed fix
- const branchMatch = text.match(/\bbranch\s+([a-zA-Z0-9._\/-]+)/i); + const branchAfterKeyword = text.match(/\bbranch\s+([a-zA-Z0-9._\/-]+)/i)?.[1]; + const branchBeforeKeyword = text.match(/\b([a-zA-Z0-9._\/-]+)\s+branch\b/i)?.[1]; + const parsedBranch = branchAfterKeyword ?? branchBeforeKeyword; + const branchStopWords = new Set(['to', 'from', 'on', 'in', 'at', 'remote']); const remote = options?.remote || remoteMatch?.[1] || 'origin'; - const branch = options?.branch || branchMatch?.[1]; + const branch = + options?.branch || + (parsedBranch && !branchStopWords.has(parsedBranch.toLowerCase()) + ? parsedBranch + : undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/push.ts` around lines 68 - 73, The branch extraction regex in push.ts (branchMatch and the subsequent branch assignment) is too permissive and picks up words like "to" in phrases such as "feature branch to origin"; update the parsing so branchMatch looks for either "branch <name>" or common phrasing where the branch name follows keywords like "feature" or "branch" but excludes prepositions — e.g., change the regex used in branchMatch to require the captured token to contain a letter or digit and not be a connector (avoid matching "to"), and then ensure the branch variable assignment (const branch = ...) prefers options?.branch, then branchMatch?.[1], and falls back to undefined; keep setUpstream logic the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/checkout.ts`:
- Around line 34-57: The parser can pick up the literal token "new" as a branch
name when scanning after a command; update the filler-word logic so "new" is
treated as a filler (add "new" to FILLER_WORDS) so the loop inside afterCommand
(the code that trims/splits afterCommand[1] and checks FILLER_WORDS/STOP_WORDS)
will skip "new" and continue to the real branch token; keep the existing
exception that preserves tokens containing branch chars (the /-.) so hyphenated
names still pass.
In `@src/actions/clone.ts`:
- Around line 55-68: extractBranch's patterns (branchMatch/onMatch) don't handle
inputs like "on the canary branch" so it fails to extract the branch name;
update the matching logic (e.g., in extractBranch) to add a pattern that allows
optional filler words like "the"/"a"/"an" between "on" and the branch name and
an optional trailing "branch" token (capture the actual name in a group), then
validate the captured name against FILLER_WORDS before returning (keep existing
branchMatch and onMatch checks but add this additional regex branch to handle
"on the <name> branch" phrasing).
In `@src/actions/log.ts`:
- Around line 46-53: The branch parser misses phrases like "from develop
branch"; update the extraction logic in the block using withBranchKeyword and
directBranch so it also matches the reversed order pattern (e.g., "from <branch>
branch")—add or extend a regex to capture from\s+([a-zA-Z0-9/_.-]+)\s+branch,
then return that capture if present and not in FILLER_WORDS; keep checks tied to
the existing symbols withBranchKeyword, directBranch and FILLER_WORDS so
behavior and validation remain consistent.
In `@src/actions/pull.ts`:
- Around line 68-72: The branch extraction currently only matches "branch
<name>" via branchMatch = text.match(/\bbranch\s+([a-zA-Z0-9._\/-]+)/i), which
misses phrases like "<name> branch"; update the parsing in src/actions/pull.ts
to also detect and prefer a trailing "branch" form (for example by trying a
second regex for /([a-zA-Z0-9._\/-]+)\s+branch\b/i or combining both patterns)
and set branch = options?.branch || branchMatch?.[1] (or the alternate match
group) so inputs like "pull from origin develop branch" correctly populate
branch instead of falling back to an unintended ref. Ensure you still honor
options?.branch and remote/remoteMatch logic.
In `@src/providers/settings.ts`:
- Line 65: The parsed GIT_CLONE_TIMEOUT may produce NaN because
cloneTimeoutSeconds is set via parseInt(String(cloneTimeout)); update the
parsing in src/providers/settings.ts so cloneTimeoutSeconds (and any other
timeout parsing) validate the result and fall back to a safe default (e.g., 0 or
a configured DEFAULT_CLONE_TIMEOUT) when parseInt yields NaN; locate the
cloneTimeout variable and the assignment to cloneTimeoutSeconds and replace the
direct parseInt with guarded logic that checks Number.isFinite/Number.isNaN (or
uses Number.isInteger) and assigns the fallback when invalid to prevent NaNs
from propagating (affects the value used later at line where NaNs appear).
---
Outside diff comments:
In `@src/actions/push.ts`:
- Around line 68-73: The branch extraction regex in push.ts (branchMatch and the
subsequent branch assignment) is too permissive and picks up words like "to" in
phrases such as "feature branch to origin"; update the parsing so branchMatch
looks for either "branch <name>" or common phrasing where the branch name
follows keywords like "feature" or "branch" but excludes prepositions — e.g.,
change the regex used in branchMatch to require the captured token to contain a
letter or digit and not be a connector (avoid matching "to"), and then ensure
the branch variable assignment (const branch = ...) prefers options?.branch,
then branchMatch?.[1], and falls back to undefined; keep setUpstream logic the
same.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (15)
.gitignorepackage.jsonsrc/actions/checkout.tssrc/actions/clone.tssrc/actions/log.tssrc/actions/pull.tssrc/actions/push.tssrc/actions/status.tssrc/index.tssrc/plugin.tssrc/providers/index.tssrc/providers/settings.tssrc/services/git.tssrc/utils/state.tstsconfig.tsbuildinfo
💤 Files with no reviewable changes (1)
- tsconfig.tsbuildinfo
Comment on lines
+34
to
+57
| const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'on', 'for']); | ||
| // Words that signal we've gone past the branch name in the sentence | ||
| const STOP_WORDS = new Set(['branch', 'and', 'then', 'first', 'now', 'from', 'called', 'named']); | ||
|
|
||
| // 1. Quoted branch names are unambiguous -- highest priority | ||
| const quotedMatch = text.match(/(?:checkout|switch|branch|create|new)\b.*?["'`]([a-zA-Z0-9._\/-]+)["'`]/i); | ||
| if (quotedMatch?.[1]) return quotedMatch[1]; | ||
|
|
||
| // 2. Find text after a command keyword, scan for first branch-like token | ||
| const afterCommand = text.match( | ||
| /(?:checkout|switch(?:\s+to)?|(?:create|new)\s+(?:branch\s+)?|branch)\s+(.*)/i | ||
| ); | ||
| if (checkoutMatch) { | ||
| return checkoutMatch[1]; | ||
| if (afterCommand?.[1]) { | ||
| for (const raw of afterCommand[1].trim().split(/\s+/)) { | ||
| const word = raw.replace(/^["'`]+|["'`]+$/g, ''); | ||
| if (!word) continue; | ||
| // Stop scanning at meta words (not part of the branch name) | ||
| if (STOP_WORDS.has(word.toLowerCase())) break; | ||
| // Skip filler words -- but NOT if they contain branch-name chars (- / .) | ||
| // so "the-great-refactor" is kept, but standalone "the" is skipped | ||
| if (FILLER_WORDS.has(word.toLowerCase()) && !/[-/.]/.test(word)) continue; | ||
| // Must be valid git branch characters | ||
| if (/^[a-zA-Z0-9._\/-]+$/.test(word)) return word; | ||
| break; // invalid chars = stop scanning |
There was a problem hiding this comment.
Parser can incorrectly choose new as branch name.
In phrases like “create a new branch called feature/user-auth” (Line 249), the current scan may return new at Line 56 before reaching the real branch token.
💡 Proposed fix
- const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'on', 'for']);
- // Words that signal we've gone past the branch name in the sentence
- const STOP_WORDS = new Set(['branch', 'and', 'then', 'first', 'now', 'from', 'called', 'named']);
+ const FILLER_WORDS = new Set([
+ 'the', 'a', 'an', 'to', 'into', 'on', 'for',
+ 'new', 'branch', 'called', 'named',
+ ]);
+ // Words that usually indicate a new clause, not a branch token
+ const STOP_WORDS = new Set(['and', 'then', 'first', 'now', 'from']);📝 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
| const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'on', 'for']); | |
| // Words that signal we've gone past the branch name in the sentence | |
| const STOP_WORDS = new Set(['branch', 'and', 'then', 'first', 'now', 'from', 'called', 'named']); | |
| // 1. Quoted branch names are unambiguous -- highest priority | |
| const quotedMatch = text.match(/(?:checkout|switch|branch|create|new)\b.*?["'`]([a-zA-Z0-9._\/-]+)["'`]/i); | |
| if (quotedMatch?.[1]) return quotedMatch[1]; | |
| // 2. Find text after a command keyword, scan for first branch-like token | |
| const afterCommand = text.match( | |
| /(?:checkout|switch(?:\s+to)?|(?:create|new)\s+(?:branch\s+)?|branch)\s+(.*)/i | |
| ); | |
| if (checkoutMatch) { | |
| return checkoutMatch[1]; | |
| if (afterCommand?.[1]) { | |
| for (const raw of afterCommand[1].trim().split(/\s+/)) { | |
| const word = raw.replace(/^["'`]+|["'`]+$/g, ''); | |
| if (!word) continue; | |
| // Stop scanning at meta words (not part of the branch name) | |
| if (STOP_WORDS.has(word.toLowerCase())) break; | |
| // Skip filler words -- but NOT if they contain branch-name chars (- / .) | |
| // so "the-great-refactor" is kept, but standalone "the" is skipped | |
| if (FILLER_WORDS.has(word.toLowerCase()) && !/[-/.]/.test(word)) continue; | |
| // Must be valid git branch characters | |
| if (/^[a-zA-Z0-9._\/-]+$/.test(word)) return word; | |
| break; // invalid chars = stop scanning | |
| const FILLER_WORDS = new Set([ | |
| 'the', 'a', 'an', 'to', 'into', 'on', 'for', | |
| 'new', 'branch', 'called', 'named', | |
| ]); | |
| // Words that usually indicate a new clause, not a branch token | |
| const STOP_WORDS = new Set(['and', 'then', 'first', 'now', 'from']); | |
| // 1. Quoted branch names are unambiguous -- highest priority | |
| const quotedMatch = text.match(/(?:checkout|switch|branch|create|new)\b.*?["'`]([a-zA-Z0-9._\/-]+)["'`]/i); | |
| if (quotedMatch?.[1]) return quotedMatch[1]; | |
| // 2. Find text after a command keyword, scan for first branch-like token | |
| const afterCommand = text.match( | |
| /(?:checkout|switch(?:\s+to)?|(?:create|new)\s+(?:branch\s+)?|branch)\s+(.*)/i | |
| ); | |
| if (afterCommand?.[1]) { | |
| for (const raw of afterCommand[1].trim().split(/\s+/)) { | |
| const word = raw.replace(/^["'`]+|["'`]+$/g, ''); | |
| if (!word) continue; | |
| // Stop scanning at meta words (not part of the branch name) | |
| if (STOP_WORDS.has(word.toLowerCase())) break; | |
| // Skip filler words -- but NOT if they contain branch-name chars (- / .) | |
| // so "the-great-refactor" is kept, but standalone "the" is skipped | |
| if (FILLER_WORDS.has(word.toLowerCase()) && !/[-/.]/.test(word)) continue; | |
| // Must be valid git branch characters | |
| if (/^[a-zA-Z0-9._\/-]+$/.test(word)) return word; | |
| break; // invalid chars = stop scanning |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/checkout.ts` around lines 34 - 57, The parser can pick up the
literal token "new" as a branch name when scanning after a command; update the
filler-word logic so "new" is treated as a filler (add "new" to FILLER_WORDS) so
the loop inside afterCommand (the code that trims/splits afterCommand[1] and
checks FILLER_WORDS/STOP_WORDS) will skip "new" and continue to the real branch
token; keep the existing exception that preserves tokens containing branch chars
(the /-.) so hyphenated names still pass.
Comment on lines
+55
to
68
| const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'for', 'this', 'that']); | ||
|
|
||
| // "on branch X", "branch X", "checkout X" | ||
| const branchMatch = text.match( | ||
| /(?:branch|checkout|switch to)\s+["\']?([a-zA-Z0-9._\/-]+)["\']?/i | ||
| /(?:branch|checkout|switch to)\s+["']?([a-zA-Z0-9._\/-]+)["']?/i | ||
| ); | ||
| if (branchMatch) { | ||
| if (branchMatch?.[1] && !FILLER_WORDS.has(branchMatch[1].toLowerCase())) { | ||
| return branchMatch[1]; | ||
| } | ||
|
|
||
| // "on X", at the end | ||
| // "on X" at the end of the message -- only if X is not a filler word | ||
| const onMatch = text.match(/\bon\s+([a-zA-Z0-9._\/-]+)\s*$/i); | ||
| if (onMatch) { | ||
| if (onMatch?.[1] && !FILLER_WORDS.has(onMatch[1].toLowerCase())) { | ||
| return onMatch[1]; |
There was a problem hiding this comment.
extractBranch misses common on the <name> branch input.
With the current patterns, Line 254’s example phrasing (“on the canary branch”) won’t extract canary, so clone may silently use default branch.
💡 Proposed fix
function extractBranch(text: string): string | undefined {
const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'for', 'this', 'that']);
+ // "on the canary branch", "from develop branch"
+ const namedBranch = text.match(/\b(?:on|from|in)\s+(?:the\s+)?([a-zA-Z0-9._\/-]+)\s+branch\b/i);
+ if (namedBranch?.[1] && !FILLER_WORDS.has(namedBranch[1].toLowerCase())) {
+ return namedBranch[1];
+ }
+
// "on branch X", "branch X", "checkout X"
const branchMatch = text.match(
/(?:branch|checkout|switch to)\s+["']?([a-zA-Z0-9._\/-]+)["']?/i
);📝 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
| const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'for', 'this', 'that']); | |
| // "on branch X", "branch X", "checkout X" | |
| const branchMatch = text.match( | |
| /(?:branch|checkout|switch to)\s+["\']?([a-zA-Z0-9._\/-]+)["\']?/i | |
| /(?:branch|checkout|switch to)\s+["']?([a-zA-Z0-9._\/-]+)["']?/i | |
| ); | |
| if (branchMatch) { | |
| if (branchMatch?.[1] && !FILLER_WORDS.has(branchMatch[1].toLowerCase())) { | |
| return branchMatch[1]; | |
| } | |
| // "on X", at the end | |
| // "on X" at the end of the message -- only if X is not a filler word | |
| const onMatch = text.match(/\bon\s+([a-zA-Z0-9._\/-]+)\s*$/i); | |
| if (onMatch) { | |
| if (onMatch?.[1] && !FILLER_WORDS.has(onMatch[1].toLowerCase())) { | |
| return onMatch[1]; | |
| const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'for', 'this', 'that']); | |
| // "on the canary branch", "from develop branch" | |
| const namedBranch = text.match(/\b(?:on|from|in)\s+(?:the\s+)?([a-zA-Z0-9._\/-]+)\s+branch\b/i); | |
| if (namedBranch?.[1] && !FILLER_WORDS.has(namedBranch[1].toLowerCase())) { | |
| return namedBranch[1]; | |
| } | |
| // "on branch X", "branch X", "checkout X" | |
| const branchMatch = text.match( | |
| /(?:branch|checkout|switch to)\s+["']?([a-zA-Z0-9._\/-]+)["']?/i | |
| ); | |
| if (branchMatch?.[1] && !FILLER_WORDS.has(branchMatch[1].toLowerCase())) { | |
| return branchMatch[1]; | |
| } | |
| // "on X" at the end of the message -- only if X is not a filler word | |
| const onMatch = text.match(/\bon\s+([a-zA-Z0-9._\/-]+)\s*$/i); | |
| if (onMatch?.[1] && !FILLER_WORDS.has(onMatch[1].toLowerCase())) { | |
| return onMatch[1]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/clone.ts` around lines 55 - 68, extractBranch's patterns
(branchMatch/onMatch) don't handle inputs like "on the canary branch" so it
fails to extract the branch name; update the matching logic (e.g., in
extractBranch) to add a pattern that allows optional filler words like
"the"/"a"/"an" between "on" and the branch name and an optional trailing
"branch" token (capture the actual name in a group), then validate the captured
name against FILLER_WORDS before returning (keep existing branchMatch and
onMatch checks but add this additional regex branch to handle "on the <name>
branch" phrasing).
Comment on lines
+46
to
53
| const withBranchKeyword = text.match(/(?:on|from|in)\s+branch\s+([a-zA-Z0-9/_.-]+)/i); | ||
| if (withBranchKeyword?.[1]) return withBranchKeyword[1]; | ||
|
|
||
| // "branch main", "branch develop" | ||
| const directBranch = text.match(/\bbranch\s+([a-zA-Z0-9/_.-]+)/i); | ||
| if (directBranch?.[1] && !FILLER_WORDS.has(directBranch[1].toLowerCase())) { | ||
| return directBranch[1]; | ||
| } |
There was a problem hiding this comment.
Branch parser does not handle from <name> branch phrasing.
The new patterns miss common input like Line 279 (“from develop branch”), so branch stays undefined and history may be fetched from the wrong branch.
💡 Proposed fix
function extractBranch(text: string): string | undefined {
const FILLER_WORDS = new Set(['the', 'a', 'an', 'to', 'into', 'on', 'for', 'this', 'that']);
+ // "from develop branch", "on main branch", "in release/v1 branch"
+ const preposedBranch = text.match(/(?:on|from|in)\s+([a-zA-Z0-9/_.-]+)\s+branch\b/i);
+ if (preposedBranch?.[1] && !FILLER_WORDS.has(preposedBranch[1].toLowerCase())) {
+ return preposedBranch[1];
+ }
+
// "on branch main", "from branch develop", "in branch feature/xyz"
const withBranchKeyword = text.match(/(?:on|from|in)\s+branch\s+([a-zA-Z0-9/_.-]+)/i);
if (withBranchKeyword?.[1]) return withBranchKeyword[1];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/log.ts` around lines 46 - 53, The branch parser misses phrases
like "from develop branch"; update the extraction logic in the block using
withBranchKeyword and directBranch so it also matches the reversed order pattern
(e.g., "from <branch> branch")—add or extend a regex to capture
from\s+([a-zA-Z0-9/_.-]+)\s+branch, then return that capture if present and not
in FILLER_WORDS; keep checks tied to the existing symbols withBranchKeyword,
directBranch and FILLER_WORDS so behavior and validation remain consistent.
Comment on lines
+68
to
72
| // Require "branch" keyword to avoid capturing random words from the message | ||
| const branchMatch = text.match(/\bbranch\s+([a-zA-Z0-9._\/-]+)/i); | ||
|
|
||
| const remote = options?.remote || remoteMatch?.[1]; | ||
| const branch = options?.branch || branchMatch?.[1]; |
There was a problem hiding this comment.
Branch extraction now misses "<name> branch" phrasing.
Line 69 only captures branch <name>, so inputs like “pull from origin develop branch” won’t set branch and may execute against an unintended ref.
💡 Proposed fix
- const branchMatch = text.match(/\bbranch\s+([a-zA-Z0-9._\/-]+)/i);
+ const branchMatch = text.match(
+ /\b(?:branch\s+([a-zA-Z0-9._\/-]+)|([a-zA-Z0-9._\/-]+)\s+branch)\b/i
+ );
const remote = options?.remote || remoteMatch?.[1];
- const branch = options?.branch || branchMatch?.[1];
+ const branch = options?.branch || branchMatch?.[1] || branchMatch?.[2];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/pull.ts` around lines 68 - 72, The branch extraction currently
only matches "branch <name>" via branchMatch =
text.match(/\bbranch\s+([a-zA-Z0-9._\/-]+)/i), which misses phrases like "<name>
branch"; update the parsing in src/actions/pull.ts to also detect and prefer a
trailing "branch" form (for example by trying a second regex for
/([a-zA-Z0-9._\/-]+)\s+branch\b/i or combining both patterns) and set branch =
options?.branch || branchMatch?.[1] (or the alternate match group) so inputs
like "pull from origin develop branch" correctly populate branch instead of
falling back to an unintended ref. Ensure you still honor options?.branch and
remote/remoteMatch logic.
| gitVersion, | ||
| allowedPath, | ||
| workingCopiesDir, | ||
| cloneTimeoutSeconds: parseInt(String(cloneTimeout)), |
There was a problem hiding this comment.
Guard timeout parsing to avoid NaN in provider output.
At Line 65, invalid GIT_CLONE_TIMEOUT values become NaN, which then surfaces in Line 97 as NaNs.
💡 Proposed fix
- cloneTimeoutSeconds: parseInt(String(cloneTimeout)),
+ cloneTimeoutSeconds: (() => {
+ const parsed = Number.parseInt(String(cloneTimeout), 10);
+ return Number.isFinite(parsed) && parsed > 0 ? parsed : 300;
+ })(),Also applies to: 97-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/providers/settings.ts` at line 65, The parsed GIT_CLONE_TIMEOUT may
produce NaN because cloneTimeoutSeconds is set via
parseInt(String(cloneTimeout)); update the parsing in src/providers/settings.ts
so cloneTimeoutSeconds (and any other timeout parsing) validate the result and
fall back to a safe default (e.g., 0 or a configured DEFAULT_CLONE_TIMEOUT) when
parseInt yields NaN; locate the cloneTimeout variable and the assignment to
cloneTimeoutSeconds and replace the direct parseInt with guarded logic that
checks Number.isFinite/Number.isNaN (or uses Number.isInteger) and assigns the
fallback when invalid to prevent NaNs from propagating (affects the value used
later at line where NaNs appear).
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
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 by CodeRabbit
New Features
Bug Fixes / Improvements
Chores