fix: correct agent resume flags#1899
fix: correct agent resume flags#1899janburzinski wants to merge 2 commits intogeneralaction:mainfrom
Conversation
Greptile SummaryThis PR fixes resume and session-ID flags across multiple agent providers. The core change moves
Confidence Score: 4/5The core argument-ordering fix is sound and well-motivated; the droid session-ID guard is correct but introduces a hardcoded provider name in shared logic. The behavioral changes are tested for goose and droid. The six other providers that received new resumeFlag values have no test coverage, making it hard to catch a flag typo or ordering bug without launching each CLI manually. The hardcoded droid guard in buildAgentCommand is a design concern that will need revisiting if another provider follows the same pattern. The buildAgentCommand guard on line 119 and the test file — the latter covers only two of the eight affected providers.
|
| Filename | Overview |
|---|---|
| src/main/core/conversations/impl/agent-command.ts | Two behavioral changes: defaultArgs moved before resume/session flags (fixes goose subcommand ordering), and else if condition hardcodes 'droid' to skip session ID on fresh starts. |
| src/shared/agent-provider-registry.ts | Resume flags added for cursor-agent, opencode, copilot, augment, goose, and kimi; droid switched from resumeFlag: '-r' to sessionIdFlag: '--session-id'; vibe initialPromptFlag set to empty string for positional prompt passing. |
| src/main/core/conversations/impl/agent-command.test.ts | Three new tests covering goose defaultArgs ordering, droid fresh session (no session ID), and droid resume (with session ID). The six newly-added resumeFlag providers have no dedicated tests. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildAgentCommand] --> B[parse CLI prefix]
B --> C[push defaultArgs]
C --> D{isResuming AND resumeFlag?}
D -- yes --> E[push resumeFlag]
E --> F{sessionIdFlag?}
F -- yes --> G[push sessionId value only]
F -- no --> H
G --> H
D -- no --> I{isResuming OR not droid\nAND sessionIdFlag?}
I -- yes --> J[push sessionIdFlag + sessionId]
I -- no --> K[skip session args]
J --> L
K --> L
H --> L{autoApprove AND autoApproveFlag?}
L -- yes --> M[push autoApproveFlag]
L -- no --> N
M --> N{not resuming AND initialPrompt\nAND not keystrokeInjection?}
N -- yes --> O[push initialPromptFlag + initialPrompt]
N -- no --> P
O --> P[push extraArgs]
P --> Q[return command + args]
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/main/core/conversations/impl/agent-command.ts:119
**Provider-specific logic hardcoded in core function**
The condition `providerId !== 'droid'` embeds Droid's unique "session ID only on resume" behaviour directly into `buildAgentCommand`. Any future provider that shares this pattern (pass `--session-id` on resume but not on first launch) would require another hardcoded branch. A cleaner approach is to surface this intent through the provider config — for example, a `sessionIdOnResume: true` field — so the function can remain provider-agnostic and new providers opt in through data rather than code changes.
### Issue 2 of 2
src/main/core/conversations/impl/agent-command.test.ts:85-116
**No test coverage for six newly added `resumeFlag` providers**
The PR adds `resumeFlag` to cursor-agent, opencode, copilot, augment, goose, and kimi, and changes `vibe`'s `initialPromptFlag` to `''`. Only goose and droid receive new tests. Without tests for the others, a typo in a flag value (e.g. `--contunue`) or an incorrect arg ordering would go unnoticed until runtime. Adding one test per affected provider for both the fresh-session and the resume path would provide the same confidence here as the droid and goose tests do.
Reviews (1): Last reviewed commit: "fix: correct agent resume flags" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| args.push(sessionId); | ||
| } | ||
| } else if (providerConfig?.sessionIdFlag) { | ||
| } else if ((isResuming || providerId !== 'droid') && providerConfig?.sessionIdFlag) { |
There was a problem hiding this comment.
The condition providerId !== 'droid' embeds Droid's unique "session ID only on resume" behaviour directly into buildAgentCommand. Any future provider that shares this pattern (pass --session-id on resume but not on first launch) would require another hardcoded branch. A cleaner approach is to surface this intent through the provider config — for example, a sessionIdOnResume: true field — so the function can remain provider-agnostic and new providers opt in through data rather than code changes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/main/core/conversations/impl/agent-command.ts
Line: 119
Comment:
**Provider-specific logic hardcoded in core function**
The condition `providerId !== 'droid'` embeds Droid's unique "session ID only on resume" behaviour directly into `buildAgentCommand`. Any future provider that shares this pattern (pass `--session-id` on resume but not on first launch) would require another hardcoded branch. A cleaner approach is to surface this intent through the provider config — for example, a `sessionIdOnResume: true` field — so the function can remain provider-agnostic and new providers opt in through data rather than code changes.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| it('puts default args before resume flags for CLIs with subcommands', () => { | ||
| const result = buildAgentCommand({ | ||
| providerId: 'goose', | ||
| providerConfig: providerConfigDefaults.goose, | ||
| sessionId: 'conv-1', | ||
| isResuming: true, | ||
| }); | ||
|
|
||
| expect(result.args).toEqual(['run', '-s', '--resume']); | ||
| }); | ||
|
|
||
| it('does not pass Droid session id on fresh sessions', () => { | ||
| const result = buildAgentCommand({ | ||
| providerId: 'droid', | ||
| providerConfig: providerConfigDefaults.droid, | ||
| initialPrompt: 'Fix the bug', | ||
| sessionId: 'conv-1', | ||
| }); | ||
|
|
||
| expect(result.args).toEqual(['Fix the bug']); | ||
| }); | ||
|
|
||
| it('passes Droid session id when resuming', () => { | ||
| const result = buildAgentCommand({ | ||
| providerId: 'droid', | ||
| providerConfig: providerConfigDefaults.droid, | ||
| sessionId: 'conv-1', | ||
| isResuming: true, | ||
| }); | ||
|
|
||
| expect(result.args).toEqual(['--session-id', 'conv-1']); | ||
| }); |
There was a problem hiding this comment.
resumeFlag providers
The PR adds resumeFlag to cursor-agent, opencode, copilot, augment, goose, and kimi, and changes vibe's initialPromptFlag to ''. Only goose and droid receive new tests. Without tests for the others, a typo in a flag value (e.g. --contunue) or an incorrect arg ordering would go unnoticed until runtime. Adding one test per affected provider for both the fresh-session and the resume path would provide the same confidence here as the droid and goose tests do.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/main/core/conversations/impl/agent-command.test.ts
Line: 85-116
Comment:
**No test coverage for six newly added `resumeFlag` providers**
The PR adds `resumeFlag` to cursor-agent, opencode, copilot, augment, goose, and kimi, and changes `vibe`'s `initialPromptFlag` to `''`. Only goose and droid receive new tests. Without tests for the others, a typo in a flag value (e.g. `--contunue`) or an incorrect arg ordering would go unnoticed until runtime. Adding one test per affected provider for both the fresh-session and the resume path would provide the same confidence here as the droid and goose tests do.
How can I resolve this? If you propose a fix, please make it concise.1 participant
summary
updated some flags for the agents