Skip to content

Commit 60d4eae

Browse files
committed
Switch to two-query merge with owner-namespaced ids
- Always query both `owner:me` and `shared:true`; merge and dedupe. - Drop the `workspaceFilter` setting. - Shared workspaces (where `ownerName != client.me.username`) use `<owner>.<workspace>.<agent>` as their environment id so they don't collide with the user's own workspaces; owned workspaces keep the legacy `<workspace>.<agent>` id.
1 parent 09b6551 commit 60d4eae

12 files changed

Lines changed: 135 additions & 68 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
- `Binary destination` can now point directly to an executable, used as-is; otherwise it is treated as a base directory
88
as before
99
- support for OAuth2
10-
- workspace listing now accepts a configurable `Workspace filter` (defaults to `owner:me`); leave it blank to include
11-
workspaces shared with the current user
10+
- workspaces shared with the current user via RBAC now appear in the workspace list alongside your own; shared
11+
workspaces are namespaced by owner (`<owner>.<workspace>.<agent>`) so they don't collide with yours
1212
- the URI handler accepts an optional `owner` query parameter to disambiguate shared workspaces
1313

1414
### Removed

src/main/kotlin/com/coder/toolbox/CoderRemoteEnvironment.kt

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,20 @@ import kotlin.time.Duration.Companion.seconds
4242

4343
private val POLL_INTERVAL = 5.seconds
4444

45+
/**
46+
* Build the environment id for a workspace/agent pair. Workspaces shared
47+
* with the current user are namespaced by owner (`<owner>.<workspace>.<agent>`)
48+
* so they don't collide with the user's own workspaces, while owned
49+
* workspaces keep the legacy `<workspace>.<agent>` id to preserve persistent
50+
* per-environment state (auto-connect, etc.).
51+
*/
52+
fun environmentId(workspace: Workspace, agent: WorkspaceAgent, currentUser: String): String =
53+
if (workspace.ownerName == currentUser) {
54+
"${workspace.name}.${agent.name}"
55+
} else {
56+
"${workspace.ownerName}.${workspace.name}.${agent.name}"
57+
}
58+
4559
/**
4660
* Represents an agent and workspace combination.
4761
*
@@ -53,10 +67,12 @@ class CoderRemoteEnvironment(
5367
internal var cli: CoderCLIManager,
5468
private var workspace: Workspace,
5569
private var agent: WorkspaceAgent,
56-
) : RemoteProviderEnvironment("${workspace.name}.${agent.name}"), BeforeConnectionHook, AfterDisconnectHook {
70+
) : RemoteProviderEnvironment(environmentId(workspace, agent, client.me.username)),
71+
BeforeConnectionHook,
72+
AfterDisconnectHook {
5773
private var environmentStatus = WorkspaceAndAgentStatus.from(workspace, agent)
5874

59-
override var name: String = "${workspace.name}.${agent.name}"
75+
override var name: String = environmentId(workspace, agent, client.me.username)
6076
private var isConnected: MutableStateFlow<Boolean> = MutableStateFlow(false)
6177
override val connectionRequest: MutableStateFlow<Boolean> = MutableStateFlow(false)
6278

src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@ class CoderRemoteProvider(
214214
.flatMap { it.agents ?: emptyList() }
215215
.distinctBy { it.name }
216216
.map { agent ->
217-
lastEnvironments.firstOrNull { it.id == "${ws.name}.${agent.name}" }
217+
val envId = environmentId(ws, agent, client.me.username)
218+
lastEnvironments.firstOrNull { it.id == envId }
218219
?.also {
219220
// If we have an environment already, update that.
220221
it.update(ws, agent)

src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -163,27 +163,46 @@ open class CoderRestClient(
163163
/**
164164
* Retrieves the available workspaces visible to the current user.
165165
*
166-
* The query string is taken from
167-
* [com.coder.toolbox.settings.ReadOnlyCoderSettings.workspaceFilter], which
168-
* defaults to `owner:me`. Users can broaden it (for example to an empty
169-
* string) to also include workspaces shared with them.
166+
* Runs two queries against the server, `owner:me` (workspaces owned by the
167+
* current user) and `shared:true` (workspaces shared with them via RBAC),
168+
* and returns the union deduplicated by workspace id. The `shared:true`
169+
* query is best-effort: if the server does not understand it (older
170+
* deployments) we log the error and return only the owned set.
170171
*
171-
* @throws [APIResponseException].
172+
* @throws [APIResponseException] when the `owner:me` query fails.
172173
*/
173174
suspend fun workspaces(): List<Workspace> {
174-
val workspacesResponse = callWithRetry {
175-
retroRestClient.workspaces(context.settingsStore.workspaceFilter)
175+
val owned = fetchWorkspaces("owner:me")
176+
val shared = try {
177+
fetchWorkspaces("shared:true")
178+
} catch (ex: APIResponseException) {
179+
context.logger.warn(ex, "Failed to list shared workspaces on $url; continuing with owned workspaces only")
180+
emptyList()
181+
}
182+
183+
if (shared.isEmpty()) return owned
184+
185+
// Dedupe by id in case the server returns a workspace in both queries
186+
// (for example if a user shares a workspace with themselves).
187+
val seen = HashSet<UUID>(owned.size + shared.size)
188+
return buildList {
189+
for (ws in owned + shared) {
190+
if (seen.add(ws.id)) add(ws)
191+
}
176192
}
177-
if (!workspacesResponse.isSuccessful) {
193+
}
194+
195+
private suspend fun fetchWorkspaces(query: String): List<Workspace> {
196+
val response = callWithRetry { retroRestClient.workspaces(query) }
197+
if (!response.isSuccessful) {
178198
throw APIResponseException(
179-
"retrieve workspaces",
199+
"retrieve workspaces matching '$query'",
180200
url,
181-
workspacesResponse.code(),
182-
workspacesResponse.parseErrorBody(moshi)
201+
response.code(),
202+
response.parseErrorBody(moshi)
183203
)
184204
}
185-
186-
return requireNotNull(workspacesResponse.body()?.workspaces) {
205+
return requireNotNull(response.body()?.workspaces) {
187206
"Successful response returned null body or workspaces"
188207
}
189208
}

src/main/kotlin/com/coder/toolbox/settings/ReadOnlyCoderSettings.kt

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,6 @@ interface ReadOnlyCoderSettings {
157157
*/
158158
val workspaceCreateUrl: String?
159159

160-
/**
161-
* The filter applied when listing workspaces, passed to the server as the
162-
* `q` query parameter. Defaults to `owner:me`. Set to an empty string to
163-
* include workspaces shared with the current user.
164-
*/
165-
val workspaceFilter: String
166-
167160
/**
168161
* The path where network information for SSH hosts are stored
169162
*/

src/main/kotlin/com/coder/toolbox/store/CoderSettingsStore.kt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ class CoderSettingsStore(
8888
get() = store[WORKSPACE_VIEW_URL]
8989
override val workspaceCreateUrl: String?
9090
get() = store[WORKSPACE_CREATE_URL]
91-
override val workspaceFilter: String
92-
get() = store[WORKSPACE_FILTER] ?: "owner:me"
9391

9492
/**
9593
* Where the specified deployment should put its data.
@@ -264,10 +262,6 @@ class CoderSettingsStore(
264262
store[PREFER_OAUTH2_IF_AVAILABLE] = preferAuthViaOAuth2.toString()
265263
}
266264

267-
fun updateWorkspaceFilter(filter: String) {
268-
store[WORKSPACE_FILTER] = filter
269-
}
270-
271265
private fun getDefaultGlobalDataDir(): Path {
272266
return when (getOS()) {
273267
OS.WINDOWS -> Paths.get(getWinAppData(), "coder-toolbox")

src/main/kotlin/com/coder/toolbox/store/StoreKeys.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ internal const val NETWORK_INFO_DIR = "networkInfoDir"
5353

5454
internal const val WORKSPACE_VIEW_URL = "workspaceViewUrl"
5555
internal const val WORKSPACE_CREATE_URL = "workspaceCreateUrl"
56-
internal const val WORKSPACE_FILTER = "workspaceFilter"
5756

5857
internal const val SSH_AUTO_CONNECT_PREFIX = "ssh_auto_connect_"
5958

src/main/kotlin/com/coder/toolbox/util/CoderProtocolHandler.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.coder.toolbox.util
22

33
import com.coder.toolbox.CoderToolboxContext
44
import com.coder.toolbox.cli.CoderCLIManager
5+
import com.coder.toolbox.environmentId
56
import com.coder.toolbox.feed.IdeFeedManager
67
import com.coder.toolbox.feed.IdeType
78
import com.coder.toolbox.models.WorkspaceAndAgentStatus
@@ -59,7 +60,7 @@ open class CoderProtocolHandler(
5960
) ?: return
6061
if (!ensureAgentIsReady(workspace, agent)) return
6162
delay(2.seconds)
62-
val environmentId = "${workspace.name}.${agent.name}"
63+
val environmentId = environmentId(workspace, agent, restClient.me.username)
6364
context.showEnvironmentPage(environmentId)
6465

6566
val productCode = params.ideProductCode()

src/main/kotlin/com/coder/toolbox/views/CoderSettingsPage.kt

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,6 @@ class CoderSettingsPage(
125125
TextType.General
126126
)
127127

128-
private val workspaceFilterField = TextField(
129-
context.i18n.ptrl("Workspace filter (leave blank to include shared workspaces)"),
130-
settings.workspaceFilter,
131-
TextType.General,
132-
)
133-
134128
private lateinit var visibilityUpdateJob: Job
135129
override val fields: StateFlow<List<UiField>> = MutableStateFlow(
136130
listOf(
@@ -140,7 +134,6 @@ class CoderSettingsPage(
140134
listOf(
141135
useAppNameField,
142136
disableAutostartField,
143-
workspaceFilterField,
144137
httpLoggingField,
145138
)
146139
),
@@ -221,7 +214,6 @@ class CoderSettingsPage(
221214
updateSshLogDir(sshLogDirField.contentState.value)
222215
updateNetworkInfoDir(networkInfoDirField.contentState.value)
223216
updateSshConfigOptions(sshExtraArgs.contentState.value)
224-
updateWorkspaceFilter(workspaceFilterField.contentState.value)
225217
}
226218
}
227219
)
@@ -296,10 +288,6 @@ class CoderSettingsPage(
296288
settings.networkInfoDir
297289
}
298290

299-
workspaceFilterField.contentState.update {
300-
settings.workspaceFilter
301-
}
302-
303291
visibilityUpdateJob = context.cs.launch(CoroutineName("Signature Verification Fallback Setting")) {
304292
disableSignatureVerificationField.checkedState.collect { state ->
305293
signatureFallbackStrategyField.visibility.update {

src/test/kotlin/com/coder/toolbox/CoderRemoteProviderTest.kt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ class CoderRemoteProviderTest {
4444
mockCli = mockk(relaxed = true)
4545
mockContext = mockk(relaxed = true)
4646
remoteProvider = CoderRemoteProvider(mockContext)
47+
// Default mocked client is the owner of every mocked workspace so the
48+
// env id stays in the legacy `<workspace>.<agent>` form. Tests that
49+
// exercise shared workspaces override `mockClient.me.username`.
50+
every { mockClient.me.username } returns "owner"
4751
}
4852

4953
@AfterTest
@@ -495,6 +499,23 @@ class CoderRemoteProviderTest {
495499
assertEquals("workspace2.mockAgent", result[1].id)
496500
}
497501

502+
@Test
503+
fun `given a workspace owned by someone else then env id is namespaced by owner`() = runTest {
504+
// given
505+
val agent = mockAgent("agent1")
506+
val resource = mockResource(agents = listOf(agent))
507+
val owned = mockWorkspace("mine", WorkspaceStatus.RUNNING, listOf(resource), ownerName = "me")
508+
val shared = mockWorkspace("shared", WorkspaceStatus.RUNNING, listOf(resource), ownerName = "coworker")
509+
every { mockClient.me.username } returns "me"
510+
coEvery { mockClient.workspaces() } returns listOf(owned, shared)
511+
512+
// when
513+
val result = remoteProvider.resolveWorkspaceEnvironments(mockClient, mockCli)
514+
515+
// then
516+
assertEquals(setOf("mine.agent1", "coworker.shared.agent1"), result.map { it.id }.toSet())
517+
}
518+
498519
// Helper functions
499520
private fun mockAgent(name: String, status: WorkspaceAgentStatus = WorkspaceAgentStatus.CONNECTED): WorkspaceAgent {
500521
return mockk {

0 commit comments

Comments
 (0)