Skip to content

Commit 2e0e45e

Browse files
feat: [ENG-2466] Add brv vc remote remove subcommand (#542)
* feat: [ENG-2466] Add brv vc remote remove subcommand * feat: [ENG-2466] fix review
1 parent 99309c3 commit 2e0e45e

8 files changed

Lines changed: 532 additions & 81 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import {Args, Command} from '@oclif/core'
2+
3+
import {type IVcRemoteResponse, VcEvents} from '../../../../shared/transport/events/vc-events.js'
4+
import {formatConnectionError, withDaemonRetry} from '../../../lib/daemon-client.js'
5+
6+
export default class VcRemoteRemove extends Command {
7+
public static args = {
8+
name: Args.string({description: 'Remote name', required: true}),
9+
}
10+
public static description = 'Remove a named remote'
11+
public static examples = [`<%= config.bin %> <%= command.id %> origin`]
12+
13+
public async run(): Promise<void> {
14+
const {args} = await this.parse(VcRemoteRemove)
15+
16+
if (args.name !== 'origin') {
17+
this.error(`Only 'origin' remote is currently supported.`)
18+
}
19+
20+
try {
21+
await withDaemonRetry(async (client) =>
22+
client.requestWithAck<IVcRemoteResponse>(VcEvents.REMOTE, {subcommand: 'remove'}),
23+
)
24+
this.log(`Remote 'origin' removed.`)
25+
} catch (error) {
26+
this.error(formatConnectionError(error))
27+
}
28+
}
29+
}

src/server/core/domain/entities/brv-config.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,19 @@ export class BrvConfig {
222222
}
223223
}
224224

225+
/**
226+
* Creates a new BrvConfig with space fields cleared, preserving all other fields.
227+
*/
228+
public withoutSpace(): BrvConfig {
229+
return new BrvConfig({
230+
...this,
231+
spaceId: undefined,
232+
spaceName: undefined,
233+
teamId: undefined,
234+
teamName: undefined,
235+
})
236+
}
237+
225238
/**
226239
* Creates a new BrvConfig with space fields replaced, preserving all other fields.
227240
*/

src/server/infra/transport/handlers/vc-handler.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,30 @@ export class VcHandler {
12191219
return {action: 'show', url: url ? maskCredentialsInUrl(url) : undefined}
12201220
}
12211221

1222+
if (data.subcommand === 'remove') {
1223+
const existingUrl = await this.gitService.getRemoteUrl({directory, remote: 'origin'})
1224+
if (!existingUrl) {
1225+
throw new VcError("No remote 'origin' to remove.", VcErrorCode.NO_REMOTE)
1226+
}
1227+
1228+
// Clear config before removing the remote so a mid-way failure leaves a retry-friendly state:
1229+
// if removeRemote throws, config is already cleared and remote is still present, so
1230+
// re-running `remove` will retry removeRemote and succeed. The reverse order leaves the
1231+
// remote gone but config stale, and the next `remove` fails with NO_REMOTE — unrecoverable.
1232+
const existingConfig = await this.projectConfigStore.read(projectPath)
1233+
if (existingConfig) {
1234+
await this.projectConfigStore.write(existingConfig.withoutSpace(), projectPath)
1235+
}
1236+
1237+
await this.gitService.removeRemote({directory, remote: 'origin'})
1238+
1239+
return {action: 'remove'}
1240+
}
1241+
1242+
if (data.subcommand !== 'add' && data.subcommand !== 'set-url') {
1243+
throw new VcError('Unknown remote subcommand.', VcErrorCode.INVALID_ACTION)
1244+
}
1245+
12221246
if (!data.url) {
12231247
throw new VcError('URL is required.', VcErrorCode.INVALID_REMOTE_URL)
12241248
}

src/shared/transport/events/vc-events.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,14 +166,22 @@ export interface IVcLogResponse {
166166
currentBranch?: string
167167
}
168168

169-
export type VcRemoteSubcommand = 'add' | 'set-url' | 'show'
169+
export type VcRemoteSubcommand = 'add' | 'remove' | 'set-url' | 'show'
170170

171171
export const VC_REMOTE_SUBCOMMANDS: readonly string[] = [
172172
'add',
173+
'remove',
173174
'set-url',
174175
'show',
175176
] satisfies readonly VcRemoteSubcommand[]
176177

178+
export const VC_REMOTE_SUBCOMMAND_REQUIRES_URL: Record<VcRemoteSubcommand, boolean> = {
179+
add: true,
180+
remove: false,
181+
'set-url': true,
182+
show: false,
183+
}
184+
177185
export function isVcRemoteSubcommand(value: string): value is VcRemoteSubcommand {
178186
return VC_REMOTE_SUBCOMMANDS.includes(value)
179187
}

src/tui/features/commands/definitions/vc-remote.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ import React from 'react'
22

33
import type {MessageActionReturn, SlashCommand} from '../../../types/commands.js'
44

5-
import {isVcRemoteSubcommand} from '../../../../shared/transport/events/vc-events.js'
5+
import {isVcRemoteSubcommand, VC_REMOTE_SUBCOMMAND_REQUIRES_URL} from '../../../../shared/transport/events/vc-events.js'
66
import {getGitRemoteBaseUrl} from '../../../lib/environment.js'
77
import {VcRemoteFlow} from '../../vc/remote/components/vc-remote-flow.js'
88
import {Args, parseReplArgs} from '../utils/arg-parser.js'
99

1010
/* eslint-disable perfectionist/sort-objects -- positional order matters: subcommand, name, url */
1111
const vcRemoteArgs = {
12-
subcommand: Args.string({description: 'Subcommand: add | set-url (omit to show current remote)'}),
12+
subcommand: Args.string({description: 'Subcommand: add | set-url | remove (omit to show current remote)'}),
1313
name: Args.string({description: 'Remote name (e.g. origin)'}),
1414
url: Args.string({description: `Remote URL (e.g. ${getGitRemoteBaseUrl()}/<team>/<space>.git)`}),
1515
}
@@ -29,16 +29,28 @@ export const vcRemoteSubCommand: SlashCommand = {
2929

3030
if (!isVcRemoteSubcommand(rawSubcommand)) {
3131
const errorMsg: MessageActionReturn = {
32-
content: `Unknown subcommand '${rawSubcommand}'. Usage: /vc remote [add|set-url] <name> <url>`,
32+
content: `Unknown subcommand '${rawSubcommand}'. Usage: /vc remote [add|set-url|remove] <name> [url]`,
3333
messageType: 'error',
3434
type: 'message',
3535
}
3636
return errorMsg
3737
}
3838

39-
if (!name || !url) {
39+
if (rawSubcommand === 'show') {
40+
return {
41+
render: ({onCancel, onComplete}) =>
42+
React.createElement(VcRemoteFlow, {onCancel, onComplete, subcommand: 'show'}),
43+
}
44+
}
45+
46+
const requiresUrl = VC_REMOTE_SUBCOMMAND_REQUIRES_URL[rawSubcommand]
47+
48+
if (!name || (requiresUrl && !url)) {
49+
const usage = requiresUrl
50+
? `Usage: /vc remote ${rawSubcommand} <name> <url>`
51+
: `Usage: /vc remote ${rawSubcommand} <name>`
4052
const errorMsg: MessageActionReturn = {
41-
content: `Usage: /vc remote ${rawSubcommand} <name> <url>`,
53+
content: usage,
4254
messageType: 'error',
4355
type: 'message',
4456
}
@@ -60,9 +72,9 @@ export const vcRemoteSubCommand: SlashCommand = {
6072
}
6173
},
6274
args: [
63-
{description: 'Subcommand: add | set-url (omit to show current remote)', name: 'subcommand'},
75+
{description: 'Subcommand: add | set-url | remove (omit to show current remote)', name: 'subcommand'},
6476
{description: 'Remote name (e.g. origin)', name: 'name'},
65-
{description: 'Remote URL', name: 'url'},
77+
{description: 'Remote URL (required for add | set-url)', name: 'url'},
6678
],
6779
description: 'Manage remote origin for ByteRover version control',
6880
name: 'remote',

src/tui/features/vc/remote/components/vc-remote-flow.tsx

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ type VcRemoteFlowProps = CustomDialogCallbacks & {
1515

1616
const LABELS: Record<VcRemoteSubcommand, string> = {
1717
add: 'Adding remote...',
18+
remove: 'Removing remote...',
1819
'set-url': 'Updating remote...',
1920
show: 'Fetching remote...',
2021
}
@@ -39,12 +40,32 @@ export function VcRemoteFlow({onCancel, onComplete, subcommand, url}: VcRemoteFl
3940
onComplete(`Failed: ${formatTransportError(error)}`)
4041
},
4142
onSuccess(result) {
42-
if (result.action === 'show') {
43-
onComplete(result.url ? `origin: ${result.url}` : 'No remote configured.')
44-
} else if (result.action === 'add') {
45-
onComplete(`Remote 'origin' set to ${result.url}.`)
46-
} else {
47-
onComplete(`Remote 'origin' updated to ${result.url}.`)
43+
switch (result.action) {
44+
case 'add': {
45+
onComplete(`Remote 'origin' set to ${result.url}.`)
46+
break
47+
}
48+
49+
case 'remove': {
50+
onComplete(`Remote 'origin' removed.`)
51+
break
52+
}
53+
54+
case 'set-url': {
55+
onComplete(`Remote 'origin' updated to ${result.url}.`)
56+
break
57+
}
58+
59+
case 'show': {
60+
onComplete(result.url ? `origin: ${result.url}` : 'No remote configured.')
61+
break
62+
}
63+
64+
default: {
65+
const exhaustive: never = result.action
66+
onComplete(`Unknown action: ${String(exhaustive)}`)
67+
break
68+
}
4869
}
4970
},
5071
},

test/unit/core/domain/entities/brv-config.test.ts

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,60 @@ describe('BrvConfig', () => {
185185
})
186186
})
187187

188+
describe('withoutSpace', () => {
189+
it('should create new config with space fields cleared, preserving all other fields', () => {
190+
const original = new BrvConfig(validConstructorArgs)
191+
const cleared = original.withoutSpace()
192+
193+
expect(cleared.spaceId).to.be.undefined
194+
expect(cleared.spaceName).to.be.undefined
195+
expect(cleared.teamId).to.be.undefined
196+
expect(cleared.teamName).to.be.undefined
197+
expect(cleared.chatLogPath).to.equal(original.chatLogPath)
198+
expect(cleared.cwd).to.equal(original.cwd)
199+
expect(cleared.ide).to.equal(original.ide)
200+
expect(cleared.createdAt).to.equal(original.createdAt)
201+
expect(cleared.version).to.equal(original.version)
202+
})
203+
204+
it('should not mutate the original config', () => {
205+
const original = new BrvConfig(validConstructorArgs)
206+
original.withoutSpace()
207+
208+
expect(original.spaceId).to.equal(validConstructorArgs.spaceId)
209+
expect(original.teamId).to.equal(validConstructorArgs.teamId)
210+
})
211+
212+
it('should produce a config that is not cloud-connected', () => {
213+
const original = new BrvConfig(validConstructorArgs)
214+
expect(original.isCloudConnected()).to.be.true
215+
expect(original.withoutSpace().isCloudConnected()).to.be.false
216+
})
217+
218+
it('should preserve non-space fields (cipherAgent fields) through withoutSpace', () => {
219+
const original = new BrvConfig({
220+
...validConstructorArgs,
221+
cipherAgentContext: 'context-payload',
222+
cipherAgentModes: ['mode-a', 'mode-b'],
223+
cipherAgentSystemPrompt: 'system-prompt-text',
224+
})
225+
const cleared = original.withoutSpace()
226+
227+
expect(cleared.cipherAgentContext).to.equal('context-payload')
228+
expect(cleared.cipherAgentModes).to.deep.equal(['mode-a', 'mode-b'])
229+
expect(cleared.cipherAgentSystemPrompt).to.equal('system-prompt-text')
230+
})
231+
})
232+
188233
describe('fromSpace', () => {
189234
it('should create config from Space entity with current version', () => {
190-
const space = new Space({id: 'space-789', isDefault: false, name: 'my-space', teamId: 'team-abc', teamName: 'my-team'})
235+
const space = new Space({
236+
id: 'space-789',
237+
isDefault: false,
238+
name: 'my-space',
239+
teamId: 'team-abc',
240+
teamName: 'my-team',
241+
})
191242

192243
const config = BrvConfig.fromSpace({
193244
chatLogPath: '/path/to/logs',
@@ -203,6 +254,5 @@ describe('BrvConfig', () => {
203254
expect(config.teamName).to.equal('my-team')
204255
expect(config.createdAt).to.be.a('string')
205256
})
206-
207257
})
208258
})

0 commit comments

Comments
 (0)