From 0903d62940e645f61de1db45c3463162a4ac36f0 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 24 Sep 2026 19:51:51 -0700 Subject: [PATCH 01/10] feat(desktop): attach stored secrets to workspaces from the Secrets page Add secret_attach/secret_detach IPC handlers driving devsy secret attach/detach, per-context inject switches on the Secrets page, and an attached field in secret list JSON mirroring env list. Covers the secrets half of #1265; attached secrets keep using the protected secret-delivery path, never --env. E2E proves a context-attached secret reaches the workspace at up and is gone after detach. --- cmd/secrets/list.go | 72 ++-- cmd/secrets/list_test.go | 58 +++ .../main/__tests__/ipc-provider-jobs.test.ts | 77 +++- desktop/src/main/ipc.ts | 375 ++++++++++++------ .../src/renderer/src/lib/ipc/commands.test.ts | 22 + desktop/src/renderer/src/lib/ipc/commands.ts | 66 ++- desktop/src/renderer/src/lib/types/index.ts | 1 + .../src/renderer/src/pages/SecretsPage.svelte | 64 ++- .../renderer/src/pages/SecretsPage.test.ts | 88 ++++ e2e/tests/up/helper.go | 49 +++ e2e/tests/up/provider_docker.go | 59 +-- .../.devcontainer.json | 4 + .../docs/developing-in-workspaces/secrets.mdx | 8 + 13 files changed, 730 insertions(+), 213 deletions(-) create mode 100644 cmd/secrets/list_test.go create mode 100644 desktop/src/renderer/src/pages/SecretsPage.test.ts create mode 100644 e2e/tests/up/testdata/docker-managed-secret-attached/.devcontainer.json diff --git a/cmd/secrets/list.go b/cmd/secrets/list.go index 0d4bb7196..ad80dabb2 100644 --- a/cmd/secrets/list.go +++ b/cmd/secrets/list.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "fmt" + "slices" "time" "github.com/devsy-org/devsy/cmd/flags" + "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/output" "github.com/devsy-org/devsy/pkg/secrets" "github.com/devsy-org/devsy/pkg/table" @@ -41,23 +43,22 @@ type secretEntry struct { Created string `json:"created,omitempty"` LastUsed string `json:"lastUsed,omitempty"` Orphaned bool `json:"orphaned,omitempty"` + Attached bool `json:"attached"` } func (cmd *ListCmd) Run(_ context.Context) error { - contextName, store, err := resolveContext(cmd.GlobalFlags) + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err } - - all, err := store.List(contextName) + contextName := devsyConfig.DefaultContext + store, err := secrets.NewStoreForConfig(devsyConfig) if err != nil { return err } - metas := make([]secrets.SecretMeta, 0, len(all)) - for _, m := range all { - if m.Sensitive() { - metas = append(metas, m) - } + entries, err := listEntries(devsyConfig, store, contextName) + if err != nil { + return err } mode, err := output.ResolveMode(cmd.ResultFormat) @@ -67,30 +68,32 @@ func (cmd *ListCmd) Run(_ context.Context) error { switch mode { case output.ModePlain: - renderPlain(metas) + renderPlain(entries) case output.ModeJSON: - return renderJSON(metas) + return renderJSON(entries) } return nil } -func renderPlain(metas []secrets.SecretMeta) { - tableEntries := [][]string{} - for _, m := range metas { - tableEntries = append(tableEntries, []string{ - m.Name, - formatTime(m.Created), - formatTime(m.LastUsed), - orphanLabel(m.Orphaned), - }) +func listEntries( + devsyConfig *config.Config, + store secrets.Store, + contextName string, +) ([]secretEntry, error) { + all, err := store.List(contextName) + if err != nil { + return nil, err } - table.Print([]string{"Name", "Created", "Last Used", "Status"}, tableEntries) -} - -func renderJSON(metas []secrets.SecretMeta) error { - entries := []secretEntry{} - for _, m := range metas { + var attachedNames []string + if ctxConfig := devsyConfig.Contexts[contextName]; ctxConfig != nil { + attachedNames = ctxConfig.Secrets + } + entries := make([]secretEntry, 0, len(all)) + for _, m := range all { + if !m.Sensitive() { + continue + } entries = append(entries, secretEntry{ Name: m.Name, Context: m.Context, @@ -98,8 +101,27 @@ func renderJSON(metas []secrets.SecretMeta) error { Created: formatTime(m.Created), LastUsed: formatTime(m.LastUsed), Orphaned: m.Orphaned, + Attached: slices.Contains(attachedNames, m.Name), + }) + } + return entries, nil +} + +func renderPlain(entries []secretEntry) { + tableEntries := [][]string{} + for _, e := range entries { + tableEntries = append(tableEntries, []string{ + e.Name, + e.Created, + e.LastUsed, + orphanLabel(e.Orphaned), + fmt.Sprint(e.Attached), }) } + table.Print([]string{"Name", "Created", "Last Used", "Status", "Attached"}, tableEntries) +} + +func renderJSON(entries []secretEntry) error { out, err := json.MarshalIndent(entries, "", " ") if err != nil { return err diff --git a/cmd/secrets/list_test.go b/cmd/secrets/list_test.go new file mode 100644 index 000000000..713d6eef2 --- /dev/null +++ b/cmd/secrets/list_test.go @@ -0,0 +1,58 @@ +package secrets + +import ( + "testing" + "time" + + "github.com/devsy-org/devsy/pkg/config" + devsysecrets "github.com/devsy-org/devsy/pkg/secrets" + "github.com/stretchr/testify/require" +) + +type listTestStore struct { + metas []devsysecrets.SecretMeta +} + +func (s *listTestStore) Set(string, string, string, devsysecrets.Kind) error { return nil } +func (s *listTestStore) Get(string, string) (string, error) { return "", nil } +func (s *listTestStore) Meta(string, string) (devsysecrets.SecretMeta, error) { + return devsysecrets.SecretMeta{}, nil +} +func (s *listTestStore) List(string) ([]devsysecrets.SecretMeta, error) { return s.metas, nil } +func (s *listTestStore) Delete(string, string) error { return nil } + +func TestListEntriesMarksAttachedSecrets(t *testing.T) { + created := time.Date(2026, 9, 24, 12, 0, 0, 0, time.UTC) + store := &listTestStore{metas: []devsysecrets.SecretMeta{ + { + Name: "ATTACHED", + Context: config.DefaultContext, + Kind: devsysecrets.KindSecret, + Created: created, + }, + {Name: "DETACHED", Context: config.DefaultContext, Kind: devsysecrets.KindSecret}, + {Name: "ENV_VAR", Context: config.DefaultContext, Kind: devsysecrets.KindEnv}, + }} + cfg := deleteTestConfig([]string{"ATTACHED", "sops:project/API_TOKEN"}) + + entries, err := listEntries(cfg, store, config.DefaultContext) + require.NoError(t, err) + require.Len(t, entries, 2) + require.Equal(t, "ATTACHED", entries[0].Name) + require.True(t, entries[0].Attached) + require.Equal(t, created.Format(time.RFC3339), entries[0].Created) + require.Equal(t, "DETACHED", entries[1].Name) + require.False(t, entries[1].Attached) +} + +func TestListEntriesDetachedWhenContextHasNoBindings(t *testing.T) { + store := &listTestStore{metas: []devsysecrets.SecretMeta{ + {Name: "TOKEN", Context: config.DefaultContext, Kind: devsysecrets.KindSecret}, + }} + cfg := deleteTestConfig(nil) + + entries, err := listEntries(cfg, store, config.DefaultContext) + require.NoError(t, err) + require.Len(t, entries, 1) + require.False(t, entries[0].Attached) +} diff --git a/desktop/src/main/__tests__/ipc-provider-jobs.test.ts b/desktop/src/main/__tests__/ipc-provider-jobs.test.ts index d1b49ef84..82d22071f 100644 --- a/desktop/src/main/__tests__/ipc-provider-jobs.test.ts +++ b/desktop/src/main/__tests__/ipc-provider-jobs.test.ts @@ -90,7 +90,13 @@ function invoke(channel: string, args: Record) { } function statusLine(phase: string) { - return JSON.stringify({ kind: "status", schemaVersion: 1, pipeline: "provider", phase, state: "started" }) + return JSON.stringify({ + kind: "status", + schemaVersion: 1, + pipeline: "provider", + phase, + state: "started", + }) } describe("provider job lifecycle over IPC", () => { @@ -99,6 +105,40 @@ describe("provider job lifecycle over IPC", () => { vi.clearAllMocks() }) + it("forwards managed secret attachment intent to the CLI", async () => { + const { cli } = setup() + await invoke("secret_attach", { name: "DB_PASSWORD", context: "staging" }) + await invoke("secret_detach", { name: "DB_PASSWORD", context: "staging" }) + expect(cli.runRaw).toHaveBeenCalledWith([ + "--context", + "staging", + "secret", + "attach", + "DB_PASSWORD", + ]) + expect(cli.runRaw).toHaveBeenCalledWith([ + "--context", + "staging", + "secret", + "detach", + "DB_PASSWORD", + ]) + }) + + it("rejects managed secret attachment without a context", async () => { + const { cli } = setup() + const result = (await invoke("secret_attach", { + name: "DB_PASSWORD", + context: "", + })) as { ok: boolean; message: string; cliError?: unknown } + expect(result).toEqual({ + ok: false, + message: "context is required", + cliError: undefined, + }) + expect(cli.runRaw).not.toHaveBeenCalled() + }) + it("forwards managed environment attachment intent to the CLI", async () => { const { cli } = setup() await invoke("env_attach", { name: "LOG_LEVEL", context: "staging" }) @@ -225,15 +265,21 @@ describe("provider job lifecycle over IPC", () => { expect(providerJobs.get("docker")?.error).toBeTruthy() }) - it("streams update phases and completes the provider job", async () => { const seen: string[][] = [] const { providerJobs, send } = setup((cliArgs) => { seen.push(cliArgs) - return { lines: [statusLine(cliArgs[1] === "init" ? "running_init" : "downloading")], code: 0 } + return { + lines: [ + statusLine(cliArgs[1] === "init" ? "running_init" : "downloading"), + ], + code: 0, + } }) - const commandId = await invoke("provider_update_streaming", { name: "docker" }) + const commandId = await invoke("provider_update_streaming", { + name: "docker", + }) expect(commandId).toEqual(expect.any(String)) expect(providerJobs.get("docker")?.activity).toBe("updating") @@ -252,18 +298,23 @@ describe("provider job lifecycle over IPC", () => { })) await invoke("provider_update_streaming", { name: "docker" }) - await vi.waitFor(() => expect(providerJobs.get("docker")?.error).toBeTruthy()) + await vi.waitFor(() => + expect(providerJobs.get("docker")?.error).toBeTruthy(), + ) }) - it("terminates a streaming update when provider refresh fails", async () => { const { providerJobs, send } = setup(() => ({ lines: [], code: 0 })) providerJobs.setRefresh(() => Promise.reject(new Error("refresh boom"))) - const commandId = await invoke("provider_update_streaming", { name: "docker" }) + const commandId = await invoke("provider_update_streaming", { + name: "docker", + }) await vi.waitFor(() => - expect(providerJobs.get("docker")?.errorCode).toBe("provider_refresh_failed"), + expect(providerJobs.get("docker")?.errorCode).toBe( + "provider_refresh_failed", + ), ) expect(send).toHaveBeenCalledWith( "command-progress", @@ -271,7 +322,6 @@ describe("provider job lifecycle over IPC", () => { ) }) - it("does not blame a successful init for a refresh failure afterward", async () => { const { providerJobs } = setup(() => ({ lines: [statusLine("running_init"), statusLine("ready")], @@ -303,7 +353,9 @@ describe("provider job lifecycle over IPC", () => { it("returns failure and retains recovery when provider refresh still fails", async () => { const { providerJobs } = setup(() => ({ lines: [], code: 0 })) - providerJobs.setRefresh(() => Promise.reject(new Error("still unavailable"))) + providerJobs.setRefresh(() => + Promise.reject(new Error("still unavailable")), + ) providerJobs.start("docker", "updating") await providerJobs.finish("docker", { code: "provider_refresh_failed", @@ -313,7 +365,8 @@ describe("provider job lifecycle over IPC", () => { const result = await invoke("provider_refresh_state", { name: "docker" }) expect(result).toEqual({ ok: false, message: "still unavailable" }) - expect(providerJobs.get("docker")?.errorCode).toBe("provider_refresh_failed") + expect(providerJobs.get("docker")?.errorCode).toBe( + "provider_refresh_failed", + ) }) - }) diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 68bfd0823..f26acea7b 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -17,10 +17,7 @@ import { loadCatalog } from "./image-catalog.js" import type { LogStore } from "./log-store.js" import type { MachineDiagnosticsStore } from "./machine-diagnostics-store.js" import type { MachineDiagnosticsManager } from "./machine-diagnostics-manager.js" -import type { - ProviderActivity, - ProviderJobs, -} from "./provider-jobs.js" +import type { ProviderActivity, ProviderJobs } from "./provider-jobs.js" import type { PtyManager } from "./pty.js" import { sanitizeAppSettingsPatch } from "./app-settings.js" import type { SettingsService } from "./settings-service.js" @@ -51,6 +48,7 @@ interface SecretEntry { lastUsed?: string orphaned?: boolean backend?: "keyring" | "file" + attached?: boolean } interface EnvEntry { @@ -106,8 +104,8 @@ interface IpcDependencies { cli: CliRunner state: DaemonState logStore: LogStore - machineDiagnosticsStore?: MachineDiagnosticsStore - machineDiagnosticsManager?: MachineDiagnosticsManager + machineDiagnosticsStore?: MachineDiagnosticsStore + machineDiagnosticsManager?: MachineDiagnosticsManager pty: PtyManager getMainWindow: () => BrowserWindow | null providerJobs: ProviderJobs @@ -142,13 +140,19 @@ interface ProgressSink { function redactSensitiveText(value: string): string { const secrets = Object.entries(process.env) - .filter(([name, secret]) => - secret && - /(PASSWORD|PASSWD|TOKEN|SECRET|API_KEY|APIKEY|AUTH|CREDENTIAL|PRIVATE_KEY|ACCESS_KEY)/i.test(name), + .filter( + ([name, secret]) => + secret && + /(PASSWORD|PASSWD|TOKEN|SECRET|API_KEY|APIKEY|AUTH|CREDENTIAL|PRIVATE_KEY|ACCESS_KEY)/i.test( + name, + ), ) .map(([, secret]) => secret as string) .sort((a, b) => b.length - a.length) - const redacted = secrets.reduce((text, secret) => text.split(secret).join("***"), value) + const redacted = secrets.reduce( + (text, secret) => text.split(secret).join("***"), + value, + ) return redacted .replace(/(https?:\/\/)[^\s/@]+@/gi, "$1***@") .replace(/(authorization\s*[:=]\s*(?:bearer|basic)\s+)[^\s,]+/gi, "$1***") @@ -184,9 +188,13 @@ function redactOperationStatus(value: OperationStatus): OperationStatus { error: value.error ? { ...value.error, - code: value.error.code ? redactSensitiveText(value.error.code) : undefined, + code: value.error.code + ? redactSensitiveText(value.error.code) + : undefined, message: redactSensitiveText(value.error.message), - hint: value.error.hint ? redactSensitiveText(value.error.hint) : undefined, + hint: value.error.hint + ? redactSensitiveText(value.error.hint) + : undefined, context: value.error.context ? Object.fromEntries( Object.entries(value.error.context).map(([key, text]) => [ @@ -237,8 +245,12 @@ function createLogSink( ...(extra ? { ...extra, - message: extra.message ? redactSensitiveText(extra.message) : undefined, - cliError: extra.cliError ? redactCLIError(extra.cliError) : undefined, + message: extra.message + ? redactSensitiveText(extra.message) + : undefined, + cliError: extra.cliError + ? redactCLIError(extra.cliError) + : undefined, } : {}), }) @@ -280,7 +292,17 @@ export function registerIpcHandlers(deps: IpcDependencies): { start: (workspaceId: string) => Promise } } { - const { cli, state, logStore, pty, providerJobs, workspaceJobs, machineDiagnosticsStore, machineDiagnosticsManager, getMainWindow } = deps + const { + cli, + state, + logStore, + pty, + providerJobs, + workspaceJobs, + machineDiagnosticsStore, + machineDiagnosticsManager, + getMainWindow, + } = deps const tunnelProcesses = new Map< string, import("node:child_process").ChildProcess @@ -360,18 +382,29 @@ export function registerIpcHandlers(deps: IpcDependencies): { tunnelProc.kill("SIGTERM") await tunnelExit // If process did not close in time, forcefully kill and suppress any late callbacks - if (!settled || (tunnelProc.exitCode === null && tunnelProc.signalCode === null)) { + if ( + !settled || + (tunnelProc.exitCode === null && tunnelProc.signalCode === null) + ) { // Suppress workspace callbacks from the onLine handler - const suppressWorkspaceFn = (tunnelProc as unknown as { _suppressWorkspaceCallbacks?: () => void })._suppressWorkspaceCallbacks + const suppressWorkspaceFn = ( + tunnelProc as unknown as { _suppressWorkspaceCallbacks?: () => void } + )._suppressWorkspaceCallbacks if (suppressWorkspaceFn) suppressWorkspaceFn() // Suppress callbacks at the readline level - const suppressFn = (tunnelProc as unknown as { _suppressCallbacks?: () => void })._suppressCallbacks + const suppressFn = ( + tunnelProc as unknown as { _suppressCallbacks?: () => void } + )._suppressCallbacks if (suppressFn) suppressFn() // Close readline interfaces - const rlStdout = (tunnelProc as unknown as { _rlStdout?: { close: () => void } })._rlStdout - const rlStderr = (tunnelProc as unknown as { _rlStderr?: { close: () => void } })._rlStderr + const rlStdout = ( + tunnelProc as unknown as { _rlStdout?: { close: () => void } } + )._rlStdout + const rlStderr = ( + tunnelProc as unknown as { _rlStderr?: { close: () => void } } + )._rlStderr if (rlStdout) rlStdout.close() if (rlStderr) rlStderr.close() @@ -428,7 +461,13 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (!workspaceJobs.owns(workspaceId, commandId)) return true const status = redactOperationStatus(normalizeOperationStatus(envelope)) workspaceJobs.progress(workspaceId, commandId, status) - deps.getMainWindow()?.webContents.send("workspace-status", { commandId, workspaceId, ...status }) + deps + .getMainWindow() + ?.webContents.send("workspace-status", { + commandId, + workspaceId, + ...status, + }) return true } @@ -455,7 +494,11 @@ export function registerIpcHandlers(deps: IpcDependencies): { : undefined await providerJobs.finish( name, - cliError ?? redactCLIError({ code: "provider_failed", message: errorMessage(error) }), + cliError ?? + redactCLIError({ + code: "provider_failed", + message: errorMessage(error), + }), ) throw error } @@ -480,7 +523,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (stream !== "stdout") return const envelope = parseCliEnvelope(line) if (envelope?.kind === "status") { - const status = redactOperationStatus(normalizeOperationStatus(envelope)) + const status = redactOperationStatus( + normalizeOperationStatus(envelope), + ) providerJobs.reportStatus(name, status) } }, @@ -546,10 +591,18 @@ export function registerIpcHandlers(deps: IpcDependencies): { // ── Workspaces ── ipcMain.handle("workspace_list", () => state.workspaceList()) - ipcMain.handle("workspace_snapshot", () => deps.workspaceSnapshot?.() ?? { - workspaces: state.workspaceList(), jobs: workspaceJobs.snapshot(), revision: workspaceJobs.revision, - }) - ipcMain.handle("workspace_refresh", (_event, args: { workspaceId: string }) => workspaceJobs.retryRefresh(args.workspaceId)) + ipcMain.handle( + "workspace_snapshot", + () => + deps.workspaceSnapshot?.() ?? { + workspaces: state.workspaceList(), + jobs: workspaceJobs.snapshot(), + revision: workspaceJobs.revision, + }, + ) + ipcMain.handle("workspace_refresh", (_event, args: { workspaceId: string }) => + workspaceJobs.retryRefresh(args.workspaceId), + ) ipcMain.handle( "workspace_status", @@ -566,7 +619,10 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (args.recovery) cliArgs.push("--recovery") const generation = workspaceJobs.generation(args.workspaceId) const raw = await cli.runRaw(cliArgs) - if (!args.recovery && generation === workspaceJobs.generation(args.workspaceId)) { + if ( + !args.recovery && + generation === workspaceJobs.generation(args.workspaceId) + ) { const status = normalizeWorkspaceStatus(raw) if (status && state.updateWorkspaceStatus(args.workspaceId, status)) { // The watcher owns renderer broadcasts; this update still keeps @@ -641,7 +697,10 @@ export function registerIpcHandlers(deps: IpcDependencies): { await providerJobs.finish( args.name, redactCLIError( - cliError ?? { code: "provider_failed", message: errorMessage(error) }, + cliError ?? { + code: "provider_failed", + message: errorMessage(error), + }, ), ) throw error @@ -706,7 +765,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (stream === "stdout") { const envelope = parseCliEnvelope(line) if (envelope?.kind === "status") { - const status = redactOperationStatus(normalizeOperationStatus(envelope)) + const status = redactOperationStatus( + normalizeOperationStatus(envelope), + ) providerJobs.reportStatus(args.name, status) return } @@ -733,10 +794,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, ), ) - const exitMsg = redactSensitiveText(formatLogLine( - `Exit code: ${code}`, - code === 0 ? "INFO" : "ERROR", - )) + const exitMsg = redactSensitiveText( + formatLogLine(`Exit code: ${code}`, code === 0 ? "INFO" : "ERROR"), + ) win?.webContents.send("command-progress", { commandId: cmdId, message: exitMsg, @@ -787,36 +847,39 @@ export function registerIpcHandlers(deps: IpcDependencies): { const runStep = (cliArgs: string[]): Promise => new Promise((resolve, reject) => { - cli.runStreaming( - cliArgs, - (line, stream, meta) => { - if (stream === "stdout") { - const envelope = parseCliEnvelope(line) - if (envelope?.kind === "status") { - providerJobs.reportStatus( - args.name, - redactOperationStatus(normalizeOperationStatus(envelope)), - ) + cli + .runStreaming( + cliArgs, + (line, stream, meta) => { + if (stream === "stdout") { + const envelope = parseCliEnvelope(line) + if (envelope?.kind === "status") { + providerJobs.reportStatus( + args.name, + redactOperationStatus(normalizeOperationStatus(envelope)), + ) + return + } + } + sendProgress(line, meta?.level) + }, + (code, cliError) => { + if (code === 0) { + resolve() return } - } - sendProgress(line, meta?.level) - }, - (code, cliError) => { - if (code === 0) { - resolve() - return - } - reject( - Object.assign( - new Error( - cliError?.message ?? `${cliArgs.join(" ")} exited with ${code}`, + reject( + Object.assign( + new Error( + cliError?.message ?? + `${cliArgs.join(" ")} exited with ${code}`, + ), + { cliError }, ), - { cliError }, - ), - ) - }, - ).catch(reject) + ) + }, + ) + .catch(reject) }) void (async () => { @@ -840,7 +903,8 @@ export function registerIpcHandlers(deps: IpcDependencies): { } catch (error) { failure = { code: "provider_refresh_failed", - message: "The provider updated, but its current state could not be refreshed.", + message: + "The provider updated, but its current state could not be refreshed.", hint: "Refresh provider status to try again.", context: { cause: errorMessage(error) }, } @@ -865,14 +929,17 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, ) - ipcMain.handle("provider_refresh_state", async (_event, args: { name: string }) => { - try { - await providerJobs.retryRefresh(args.name) - return { ok: true } as const - } catch (error) { - return { ok: false, message: errorMessage(error) } as const - } - }) + ipcMain.handle( + "provider_refresh_state", + async (_event, args: { name: string }) => { + try { + await providerJobs.retryRefresh(args.name) + return { ok: true } as const + } catch (error) { + return { ok: false, message: errorMessage(error) } as const + } + }, + ) ipcMain.handle("provider_options", async (_event, args: { name: string }) => { return cli.run(["provider", "get", args.name]) @@ -1022,7 +1089,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { const cliArgs = ["machine", "delete", args.id, "--context", key.context] if (args.force) cliArgs.push("--force") await cli.runRaw(cliArgs) - machineDiagnosticsManager?.delete(key) + machineDiagnosticsManager?.delete(key) }, ) @@ -1033,7 +1100,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { ipcMain.handle("machine_stop", async (_event, args: { id: string }) => { const key = { context: state.currentContext(), machineId: args.id } await cli.runRaw(["machine", "stop", args.id, "--context", key.context]) - machineDiagnosticsManager?.markStopped(key) + machineDiagnosticsManager?.markStopped(key) }) ipcMain.handle("machine_status", async (_event, args: { id: string }) => { @@ -1041,16 +1108,29 @@ export function registerIpcHandlers(deps: IpcDependencies): { }) ipcMain.handle("machine_diagnostics_get", (_event, args: { id: string }) => { - return machineDiagnosticsManager?.getCached({ context: state.currentContext(), machineId: args.id }) ?? null + return ( + machineDiagnosticsManager?.getCached({ + context: state.currentContext(), + machineId: args.id, + }) ?? null + ) }) - ipcMain.handle("machine_diagnostics_refresh", async (_event, args: { id: string }) => { - if (!machineDiagnosticsManager) throw new Error("machine diagnostics manager is unavailable") - const key = { context: state.currentContext(), machineId: args.id } - const merged = await machineDiagnosticsManager.refresh(key) - getMainWindow()?.webContents.send("machine-diagnostics-changed", { machineId: args.id, context: key.context, diagnostics: merged }) - return merged - }) + ipcMain.handle( + "machine_diagnostics_refresh", + async (_event, args: { id: string }) => { + if (!machineDiagnosticsManager) + throw new Error("machine diagnostics manager is unavailable") + const key = { context: state.currentContext(), machineId: args.id } + const merged = await machineDiagnosticsManager.refresh(key) + getMainWindow()?.webContents.send("machine-diagnostics-changed", { + machineId: args.id, + context: key.context, + diagnostics: merged, + }) + return merged + }, + ) // ── Contexts ── ipcMain.handle("context_list", () => state.contextList()) @@ -1130,6 +1210,50 @@ export function registerIpcHandlers(deps: IpcDependencies): { } }) + ipcMain.handle( + "secret_attach", + async (_event, args: { name: string; context: string }) => { + trackEvent("secret_attach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "secret", + "attach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) + + ipcMain.handle( + "secret_detach", + async (_event, args: { name: string; context: string }) => { + trackEvent("secret_detach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "secret", + "detach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) + ipcMain.handle("env_list", async () => cli.run(["env", "list"])) // Returns an envelope rather than throwing so a structured cliError survives @@ -1155,7 +1279,13 @@ export function registerIpcHandlers(deps: IpcDependencies): { trackEvent("env_delete") try { if (!args.context) throw new Error("context is required") - await cli.runRaw(["--context", args.context, "env", "delete", args.name]) + await cli.runRaw([ + "--context", + args.context, + "env", + "delete", + args.name, + ]) return { ok: true } as const } catch (err) { const cliError = (err as { cliError?: CLIError }).cliError @@ -1165,31 +1295,49 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, ) - ipcMain.handle("env_attach", async (_event, args: { name: string; context: string }) => { - trackEvent("env_attach") - try { - if (!args.context) throw new Error("context is required") - await cli.runRaw(["--context", args.context, "env", "attach", args.name]) - return { ok: true } as const - } catch (err) { - const cliError = (err as { cliError?: CLIError }).cliError - const message = err instanceof Error ? err.message : String(err) - return { ok: false, message, cliError } as const - } - }) + ipcMain.handle( + "env_attach", + async (_event, args: { name: string; context: string }) => { + trackEvent("env_attach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "env", + "attach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) - ipcMain.handle("env_detach", async (_event, args: { name: string; context: string }) => { - trackEvent("env_detach") - try { - if (!args.context) throw new Error("context is required") - await cli.runRaw(["--context", args.context, "env", "detach", args.name]) - return { ok: true } as const - } catch (err) { - const cliError = (err as { cliError?: CLIError }).cliError - const message = err instanceof Error ? err.message : String(err) - return { ok: false, message, cliError } as const - } - }) + ipcMain.handle( + "env_detach", + async (_event, args: { name: string; context: string }) => { + trackEvent("env_detach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "env", + "detach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) // ── System ── ipcMain.handle("devsy_version", async () => { @@ -1475,9 +1623,10 @@ export function registerIpcHandlers(deps: IpcDependencies): { }) return { commandId: cmdId, completion } } - - ipcMain.handle("workspace_up", (_event, args: Parameters[0]) => - runWorkspaceUp(args).commandId, + ipcMain.handle( + "workspace_up", + (_event, args: Parameters[0]) => + runWorkspaceUp(args).commandId, ) async function reconcileDetachedTask( @@ -2035,14 +2184,14 @@ export function registerIpcHandlers(deps: IpcDependencies): { runInitialProviderUpdateCheck: runUpdateCheck, workspaceActions: { async stop(workspaceId: string): Promise { - const { completion } = await startWorkspaceStop( - { workspaceId }, - "tray", - ) + const { completion } = await startWorkspaceStop({ workspaceId }, "tray") await completion }, async start(workspaceId: string): Promise { - await runWorkspaceUp({ source: workspaceId, commandId: crypto.randomUUID() }).completion + await runWorkspaceUp({ + source: workspaceId, + commandId: crypto.randomUUID(), + }).completion }, }, } diff --git a/desktop/src/renderer/src/lib/ipc/commands.test.ts b/desktop/src/renderer/src/lib/ipc/commands.test.ts index 3fd4e0c64..1e454ab5a 100644 --- a/desktop/src/renderer/src/lib/ipc/commands.test.ts +++ b/desktop/src/renderer/src/lib/ipc/commands.test.ts @@ -10,6 +10,8 @@ import { envDelete, envAttach, envDetach, + secretAttach, + secretDetach, machineCreate, machineDelete, machineStatus, @@ -145,6 +147,26 @@ describe("IPC commands", () => { }) }) + describe("managed secret commands", () => { + it("secretAttach sends the secret name and context", async () => { + mockInvoke.mockResolvedValue({ ok: true }) + await secretAttach("DB_PASSWORD", "staging") + expect(mockInvoke).toHaveBeenCalledWith("secret_attach", { + name: "DB_PASSWORD", + context: "staging", + }) + }) + + it("secretDetach sends the secret name and context", async () => { + mockInvoke.mockResolvedValue({ ok: true }) + await secretDetach("DB_PASSWORD", "staging") + expect(mockInvoke).toHaveBeenCalledWith("secret_detach", { + name: "DB_PASSWORD", + context: "staging", + }) + }) + }) + describe("managed environment commands", () => { it("envAttach sends the variable name and context", async () => { mockInvoke.mockResolvedValue({ ok: true }) diff --git a/desktop/src/renderer/src/lib/ipc/commands.ts b/desktop/src/renderer/src/lib/ipc/commands.ts index cdab0d0df..a4bcd7e3c 100644 --- a/desktop/src/renderer/src/lib/ipc/commands.ts +++ b/desktop/src/renderer/src/lib/ipc/commands.ts @@ -19,7 +19,11 @@ import type { MachineDiagnosticsCache } from "$shared/machine-diagnostics-types. type CommandEnvelope = | { ok: true } - | { ok: false; message: string; cliError?: import("$shared/cli-error.js").CLIError } + | { + ok: false + message: string + cliError?: import("$shared/cli-error.js").CLIError + } /** Unwrap a structured command envelope, rethrowing failures as an Error with .cliError attached. */ function unwrapEnvelope(result: CommandEnvelope): void { @@ -166,7 +170,9 @@ export async function providerUpdateStreaming(name: string): Promise { } export async function providerRefreshState(name: string): Promise { - unwrapEnvelope(await invoke("provider_refresh_state", { name })) + unwrapEnvelope( + await invoke("provider_refresh_state", { name }), + ) } export async function providerOptions( @@ -193,18 +199,24 @@ export async function providerRename( } export async function providerListVersions(name: string, noCache?: boolean) { - return invoke<{ versions: ProviderVersion[]; unsupported: boolean; error?: string }>( - "provider_list_versions", - { name, noCache }, - ) + return invoke<{ + versions: ProviderVersion[] + unsupported: boolean + error?: string + }>("provider_list_versions", { name, noCache }) } -export async function providerSetVersion(name: string, tag: string): Promise { +export async function providerSetVersion( + name: string, + tag: string, +): Promise { return invoke("provider_set_version", { name, tag }) } export async function providerCheckUpdates() { - return invoke>("provider_check_updates") + return invoke>( + "provider_check_updates", + ) } export async function providerGetUpdateCache() { @@ -271,11 +283,17 @@ export async function machineStatus(id: string): Promise { } } -export async function machineDiagnosticsGet(id: string): Promise { - return invoke("machine_diagnostics_get", { id }) +export async function machineDiagnosticsGet( + id: string, +): Promise { + return invoke("machine_diagnostics_get", { + id, + }) } -export async function machineDiagnosticsRefresh(id: string): Promise { +export async function machineDiagnosticsRefresh( + id: string, +): Promise { return invoke("machine_diagnostics_refresh", { id }) } @@ -324,6 +342,24 @@ export async function secretDelete(name: string): Promise { unwrapEnvelope(await invoke("secret_delete", { name })) } +export async function secretAttach( + name: string, + context: string, +): Promise { + unwrapEnvelope( + await invoke("secret_attach", { name, context }), + ) +} + +export async function secretDetach( + name: string, + context: string, +): Promise { + unwrapEnvelope( + await invoke("secret_detach", { name, context }), + ) +} + export async function envList(): Promise { return invoke("env_list") } @@ -333,9 +369,7 @@ export async function envSet(name: string, value: string): Promise { } export async function envDelete(name: string, context: string): Promise { - unwrapEnvelope( - await invoke("env_delete", { name, context }), - ) + unwrapEnvelope(await invoke("env_delete", { name, context })) } export async function envAttach(name: string, context: string): Promise { @@ -422,7 +456,9 @@ export async function getReleaseChannel(): Promise { return invoke("get_release_channel") } -export async function setReleaseChannel(channel: ReleaseChannel): Promise { +export async function setReleaseChannel( + channel: ReleaseChannel, +): Promise { return invoke("set_release_channel", { channel }) } diff --git a/desktop/src/renderer/src/lib/types/index.ts b/desktop/src/renderer/src/lib/types/index.ts index 3632667a9..c176968e6 100644 --- a/desktop/src/renderer/src/lib/types/index.ts +++ b/desktop/src/renderer/src/lib/types/index.ts @@ -156,6 +156,7 @@ export interface Secret { lastUsed?: string orphaned?: boolean backend?: "keyring" | "file" + attached?: boolean } export interface EnvVar { diff --git a/desktop/src/renderer/src/pages/SecretsPage.svelte b/desktop/src/renderer/src/pages/SecretsPage.svelte index 75f4f0e6d..de1cc570e 100644 --- a/desktop/src/renderer/src/pages/SecretsPage.svelte +++ b/desktop/src/renderer/src/pages/SecretsPage.svelte @@ -3,14 +3,26 @@ import { KeyRound, Plus, Search, Trash2 } from "@lucide/svelte" import { Button } from "$lib/components/ui/button/index.js" import { Input } from "$lib/components/ui/input/index.js" import { Label } from "$lib/components/ui/label/index.js" +import { Switch } from "$lib/components/ui/switch/index.js" import { badgeVariants } from "$lib/components/ui/badge/index.js" import * as Dialog from "$lib/components/ui/dialog/index.js" import ConfirmDialog from "$lib/components/layout/ConfirmDialog.svelte" import CardSkeleton from "$lib/components/ui/skeleton/CardSkeleton.svelte" -import { secrets, secretsError, secretsLoading, refreshSecrets } from "$lib/stores/secrets.js" -import { secretSet, secretDelete } from "$lib/ipc/commands.js" +import { + secrets, + secretsError, + secretsLoading, + refreshSecrets, +} from "$lib/stores/secrets.js" +import { + secretSet, + secretDelete, + secretAttach, + secretDetach, +} from "$lib/ipc/commands.js" import { toasts } from "$lib/stores/toasts.js" import { extractErrorMessage } from "$lib/utils/error.js" +import type { Secret } from "$lib/types/index.js" const NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/ @@ -22,6 +34,8 @@ let saving = $state(false) let confirmDeleteOpen = $state(false) let pendingDelete = $state("") let deleting = $state(false) +let updatingAttachment = $state>({}) +let attachmentErrors = $state>({}) let searchTerm = $state("") let filteredSecrets = $derived.by(() => { @@ -34,9 +48,7 @@ let filteredSecrets = $derived.by(() => { }) let nameValid = $derived(NAME_PATTERN.test(newName)) -let nameExists = $derived( - $secrets.some((s) => s.name === newName.trim()), -) +let nameExists = $derived($secrets.some((s) => s.name === newName.trim())) $effect(() => { if (!createDialogOpen) { @@ -58,11 +70,29 @@ async function handleCreate() { return } createDialogOpen = false - toasts.success(replacing ? `Secret "${name}" replaced` : `Secret "${name}" saved`) + toasts.success( + replacing ? `Secret "${name}" replaced` : `Secret "${name}" saved`, + ) saving = false await refreshSecrets().catch(() => {}) } +async function setAttached(secret: Secret, attached: boolean) { + const key = `${secret.context}\x00${secret.name}` + if (updatingAttachment[key]) return + updatingAttachment = { ...updatingAttachment, [key]: true } + attachmentErrors = { ...attachmentErrors, [key]: "" } + try { + if (attached) await secretAttach(secret.name, secret.context) + else await secretDetach(secret.name, secret.context) + await refreshSecrets() + } catch (err) { + attachmentErrors = { ...attachmentErrors, [key]: extractErrorMessage(err) } + } finally { + updatingAttachment = { ...updatingAttachment, [key]: false } + } +} + function requestDelete(e: Event, name: string) { e.stopPropagation() pendingDelete = name @@ -184,7 +214,7 @@ async function confirmDelete() { {:else}
- {#each filteredSecrets as secret (secret.name)} + {#each filteredSecrets as secret (`${secret.context}\x00${secret.name}`)}
@@ -200,9 +230,23 @@ async function confirmDelete() {
-

- Context: {secret.context} -

+
+
+

Inject into workspaces

+

Context: {secret.context}. Delivered through the protected secret path when a workspace starts or is recreated.

+
+ setAttached(secret, checked)} + /> +
+ {#if attachmentErrors[`${secret.context}\x00${secret.name}`]} +

+ Failed to update attachment: {attachmentErrors[`${secret.context}\x00${secret.name}`]} +

+ {/if}
{/each}
diff --git a/desktop/src/renderer/src/pages/SecretsPage.test.ts b/desktop/src/renderer/src/pages/SecretsPage.test.ts new file mode 100644 index 000000000..44a1c3b37 --- /dev/null +++ b/desktop/src/renderer/src/pages/SecretsPage.test.ts @@ -0,0 +1,88 @@ +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/svelte" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" + +const mocks = vi.hoisted(() => ({ + secretAttach: vi.fn().mockResolvedValue(undefined), + secretDetach: vi.fn().mockResolvedValue(undefined), + secretDelete: vi.fn().mockResolvedValue(undefined), + refreshSecrets: vi.fn().mockResolvedValue(undefined), +})) +vi.mock("$lib/ipc/commands.js", () => ({ + secretAttach: mocks.secretAttach, + secretDelete: mocks.secretDelete, + secretDetach: mocks.secretDetach, + secretSet: vi.fn(), +})) +vi.mock("$lib/stores/secrets.js", async () => { + const { writable } = await import("svelte/store") + return { + secretsLoading: writable(false), + secretsError: writable(null), + secrets: writable([ + { name: "ATTACHED", context: "default", attached: true }, + { name: "DETACHED", context: "default", attached: false }, + { name: "STAGING_ONLY", context: "staging", attached: false }, + ]), + refreshSecrets: mocks.refreshSecrets, + } +}) + +import SecretsPage from "./SecretsPage.svelte" + +describe("SecretsPage managed secret attachments", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + afterEach(() => { + cleanup() + }) + + it("renders attachment state and sends attach/detach intent", async () => { + render(SecretsPage) + + expect( + screen + .getByRole("switch", { name: "Inject ATTACHED into workspaces" }) + .getAttribute("aria-checked"), + ).toBe("true") + const detached = screen.getByRole("switch", { + name: "Inject DETACHED into workspaces", + }) + expect(detached.getAttribute("aria-checked")).toBe("false") + + await fireEvent.click(detached) + await waitFor(() => + expect(mocks.secretAttach).toHaveBeenCalledWith("DETACHED", "default"), + ) + expect(mocks.refreshSecrets).toHaveBeenCalled() + + await fireEvent.click( + screen.getByRole("switch", { name: "Inject ATTACHED into workspaces" }), + ) + await waitFor(() => + expect(mocks.secretDetach).toHaveBeenCalledWith("ATTACHED", "default"), + ) + }) + + it("uses the context displayed on a stale row", async () => { + render(SecretsPage) + + await fireEvent.click( + screen.getByRole("switch", { + name: "Inject STAGING_ONLY into workspaces", + }), + ) + await waitFor(() => + expect(mocks.secretAttach).toHaveBeenCalledWith( + "STAGING_ONLY", + "staging", + ), + ) + }) +}) diff --git a/e2e/tests/up/helper.go b/e2e/tests/up/helper.go index 7cd789f04..4bc311795 100644 --- a/e2e/tests/up/helper.go +++ b/e2e/tests/up/helper.go @@ -17,6 +17,7 @@ import ( provider2 "github.com/devsy-org/devsy/pkg/provider" "github.com/devsy-org/devsy/pkg/scanner" "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" ) const ( @@ -265,3 +266,51 @@ func setupWorkspaceAndUp( } return tempDir, f.DevsyUp(ctx, append([]string{tempDir}, args...)...) } + +type contextAttachmentCase struct { + contextPrefix string + testdataDir string + store func(context.Context, string, string) + command string + name string + checkFile string +} + +// verifyContextAttachment proves a context-attached managed value reaches the +// workspace at up and is gone after detach and recreate. +func (dtc *dockerTestContext) verifyContextAttachment( + ctx context.Context, + tc contextAttachmentCase, +) { + useFileSecretsBackend() + contextName := fmt.Sprintf("%s-%d", tc.contextPrefix, time.Now().UnixNano()) + framework.ExpectNoError(dtc.f.DevsyContextCreate(ctx, contextName)) + ginkgo.DeferCleanup(func(cleanupCtx context.Context) { + _ = dtc.f.DevsyContextUse(cleanupCtx, "default") + _ = dtc.f.DevsyContextDelete(cleanupCtx, contextName) + }) + framework.ExpectNoError(dtc.f.DevsyContextUse(ctx, contextName)) + framework.ExpectNoError( + dtc.f.DevsyProviderAdd(ctx, "docker", "-o", "DOCKER_PATH=docker"), + ) + framework.ExpectNoError(dtc.f.DevsyProviderUse(ctx, "docker")) + + tempDir, err := setupWorkspace(tc.testdataDir, dtc.initialDir, dtc.f) + framework.ExpectNoError(err) + tc.store(ctx, tc.name, "expected-value") + _, err = dtc.f.ExecCommandOutput(ctx, []string{tc.command, "attach", tc.name}) + framework.ExpectNoError(err) + + // Intentionally no --env/--secret argument: the context binding is the source. + framework.ExpectNoError(dtc.f.DevsyUp(ctx, tempDir)) + out, err := dtc.execSSH(ctx, tempDir, "cat "+tc.checkFile) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("expected-value")) + + _, err = dtc.f.ExecCommandOutput(ctx, []string{tc.command, "detach", tc.name}) + framework.ExpectNoError(err) + framework.ExpectNoError(dtc.f.DevsyUpRecreate(ctx, tempDir)) + out, err = dtc.execSSH(ctx, tempDir, "cat "+tc.checkFile) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(out)).To(gomega.BeEmpty()) +} diff --git a/e2e/tests/up/provider_docker.go b/e2e/tests/up/provider_docker.go index 9a0197dc2..f7a822d0a 100644 --- a/e2e/tests/up/provider_docker.go +++ b/e2e/tests/up/provider_docker.go @@ -768,46 +768,29 @@ var _ = ginkgo.Describe( ginkgo.It( "context-attached managed env var injects without --env and is removed after detach", func(ctx context.Context) { - useFileSecretsBackend() - contextName := fmt.Sprintf("managed-env-%d", time.Now().UnixNano()) - framework.ExpectNoError(dtc.f.DevsyContextCreate(ctx, contextName)) - ginkgo.DeferCleanup(func(cleanupCtx context.Context) { - _ = dtc.f.DevsyContextUse(cleanupCtx, "default") - _ = dtc.f.DevsyContextDelete(cleanupCtx, contextName) + dtc.verifyContextAttachment(ctx, contextAttachmentCase{ + contextPrefix: "managed-env", + testdataDir: "tests/up/testdata/docker-managed-env-attached", + store: dtc.storeEnv, + command: envCmd, + name: "ATTACHED_ENV", + checkFile: "/tmp/attached-env-check.out", }) - framework.ExpectNoError(dtc.f.DevsyContextUse(ctx, contextName)) - framework.ExpectNoError( - dtc.f.DevsyProviderAdd( - ctx, - "docker", - "-o", - "DOCKER_PATH=docker", - ), - ) - framework.ExpectNoError(dtc.f.DevsyProviderUse(ctx, "docker")) - - tempDir, err := setupWorkspace( - "tests/up/testdata/docker-managed-env-attached", - dtc.initialDir, - dtc.f, - ) - framework.ExpectNoError(err) - dtc.storeEnv(ctx, "ATTACHED_ENV", "expected-value") - _, err = dtc.f.ExecCommandOutput(ctx, []string{envCmd, "attach", "ATTACHED_ENV"}) - framework.ExpectNoError(err) - - // Intentionally no --env argument: the context binding is the source. - framework.ExpectNoError(dtc.f.DevsyUp(ctx, tempDir)) - out, err := dtc.execSSH(ctx, tempDir, "cat /tmp/attached-env-check.out") - framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("expected-value")) + }, + ginkgo.SpecTimeout(framework.TimeoutShort()), + ) - _, err = dtc.f.ExecCommandOutput(ctx, []string{envCmd, "detach", "ATTACHED_ENV"}) - framework.ExpectNoError(err) - framework.ExpectNoError(dtc.f.DevsyUpRecreate(ctx, tempDir)) - out, err = dtc.execSSH(ctx, tempDir, "cat /tmp/attached-env-check.out") - framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.BeEmpty()) + ginkgo.It( + "context-attached managed secret injects without --secret and is removed after detach", + func(ctx context.Context) { + dtc.verifyContextAttachment(ctx, contextAttachmentCase{ + contextPrefix: "managed-secret", + testdataDir: "tests/up/testdata/docker-managed-secret-attached", + store: dtc.storeSecret, + command: secretCmd, + name: "ATTACHED_SECRET", + checkFile: "/tmp/attached-secret-check.out", + }) }, ginkgo.SpecTimeout(framework.TimeoutShort()), ) diff --git a/e2e/tests/up/testdata/docker-managed-secret-attached/.devcontainer.json b/e2e/tests/up/testdata/docker-managed-secret-attached/.devcontainer.json new file mode 100644 index 000000000..f8a019893 --- /dev/null +++ b/e2e/tests/up/testdata/docker-managed-secret-attached/.devcontainer.json @@ -0,0 +1,4 @@ +{ + "image": "ghcr.io/devsy-org/test-images/go:1", + "postCreateCommand": "printf '%s' \"${ATTACHED_SECRET-}\" > /tmp/attached-secret-check.out" +} diff --git a/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx b/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx index ad6fdd21f..c130c59e3 100644 --- a/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx +++ b/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx @@ -262,6 +262,10 @@ devsy secret detach DB_PASSWORD devsy secret detach sops:project/API_TOKEN ``` +Attachments are context-scoped and contain only the secret reference, never +the value. The Desktop Secrets page shows and changes the same attachment +state. + Devsy stores only the reference for an external secret. Repository-owned `customizations.devsy` can declare project-specific automatic bindings with its `secrets` list. If any requested or attached value cannot be @@ -378,3 +382,7 @@ detached before converting it into an environment variable. This keeps the non-sensitive environment path separate from protected secret delivery. If a stored value is deleted, Devsy removes its context attachment as part of the same operation. + +Sensitive values use `devsy secret attach` instead; attached secrets are +injected through the protected secret-delivery paths described above, and the +Desktop Secrets page manages their attachment state. From 0ef70c3449464ca2afc0d0bd2e1bc772cde5f475 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 24 Sep 2026 19:51:55 -0700 Subject: [PATCH 02/10] fix(config): serialize all config.yaml read-modify-write cycles under LockConfig Add config.UpdateConfig (lock, load, mutate, save) and migrate every unlocked SaveConfig caller: context create/delete/use/set-options, ide set/use, provider add/delete/init/set/set-source/use/rename (whole transaction), workspace import, pro login/logout/update-provider, the MCP provider tools, and the up-triggered provider auto-update (reload under lock). Resolves the CodeRabbit finding from #1266. --- cmd/context/create.go | 45 ++++++++++++++------------------ cmd/context/delete.go | 42 ++++++++++++++--------------- cmd/context/set_options.go | 37 ++++++++++---------------- cmd/context/use.go | 31 +++++++++------------- cmd/ide/set.go | 16 +++--------- cmd/ide/use.go | 25 +++++++----------- cmd/mcp/tools_provider.go | 11 ++++---- cmd/pro/login.go | 6 +++++ cmd/pro/logout.go | 13 +++++++-- cmd/pro/update_provider.go | 6 +++++ cmd/provider/add.go | 6 +++++ cmd/provider/delete.go | 6 +++++ cmd/provider/init.go | 6 +++++ cmd/provider/rename.go | 8 ++++++ cmd/provider/set.go | 6 +++++ cmd/provider/set_source.go | 6 +++++ cmd/provider/use.go | 43 ++++++++++++++---------------- cmd/workspace/import.go | 6 +++++ pkg/config/config.go | 19 ++++++++++++++ pkg/workspace/provider_update.go | 12 +++++++++ 20 files changed, 200 insertions(+), 150 deletions(-) diff --git a/cmd/context/create.go b/cmd/context/create.go index 91b891e5c..0a36ba6e2 100644 --- a/cmd/context/create.go +++ b/cmd/context/create.go @@ -46,36 +46,29 @@ func NewCreateCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *CreateCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig("", cmd.Provider) - if err != nil { - return err - } else if devsyConfig.Contexts[context] != nil { - return fmt.Errorf("context %q already exists", context) - } - - // verify name - if provider2.ProviderNameRegEx.MatchString(context) { - return fmt.Errorf("context name can only include lower case letters, numbers or dashes") - } else if len(context) > 48 { - return fmt.Errorf("context name cannot be longer than 48 characters") - } - devsyConfig.Contexts[context] = &config.ContextConfig{} + return config.UpdateConfig("", cmd.Provider, func(devsyConfig *config.Config) error { + if devsyConfig.Contexts[context] != nil { + return fmt.Errorf("context %q already exists", context) + } - // check if there are create options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, context, cmd.Options) - if err != nil { - return err + // verify name + if provider2.ProviderNameRegEx.MatchString(context) { + return fmt.Errorf("context name can only include lower case letters, numbers or dashes") + } else if len(context) > 48 { + return fmt.Errorf("context name cannot be longer than 48 characters") } - } + devsyConfig.Contexts[context] = &config.ContextConfig{} - devsyConfig.DefaultContext = context - err = config.SaveConfig(devsyConfig) - if err != nil { - return fmt.Errorf("save config: %w", err) - } + // check if there are create options set + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, context, cmd.Options); err != nil { + return err + } + } - return nil + devsyConfig.DefaultContext = context + return nil + }) } func setOptions(devsyConfig *config.Config, context string, options []string) error { diff --git a/cmd/context/delete.go b/cmd/context/delete.go index d622e50d3..b5ec4d0ad 100644 --- a/cmd/context/delete.go +++ b/cmd/context/delete.go @@ -44,31 +44,27 @@ func NewDeleteCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *DeleteCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig(context, cmd.Provider) - if err != nil { - return err - } - - if context == "" { - context = devsyConfig.DefaultContext - } else if devsyConfig.Contexts[context] == nil { - return fmt.Errorf("context %q doesn't exist", context) - } - - if context == "default" { - return fmt.Errorf("cannot delete 'default' context") - } + err := config.UpdateConfig(context, cmd.Provider, func(devsyConfig *config.Config) error { + if context == "" { + context = devsyConfig.DefaultContext + } else if devsyConfig.Contexts[context] == nil { + return fmt.Errorf("context %q doesn't exist", context) + } - if err := deleteContextSecrets(devsyConfig, context); err != nil { - return err - } + if context == config.DefaultContext { + return fmt.Errorf("cannot delete 'default' context") + } - delete(devsyConfig.Contexts, context) - resetContextReferences(devsyConfig, context) + if err := deleteContextSecrets(devsyConfig, context); err != nil { + return err + } - err = config.SaveConfig(devsyConfig) + delete(devsyConfig.Contexts, context) + resetContextReferences(devsyConfig, context) + return nil + }) if err != nil { - return fmt.Errorf("save config: %w", err) + return err } return removeContextDir(context) @@ -91,10 +87,10 @@ func removeContextDir(contextName string) error { func resetContextReferences(devsyConfig *config.Config, context string) { if devsyConfig.DefaultContext == context { - devsyConfig.DefaultContext = "default" + devsyConfig.DefaultContext = config.DefaultContext } if devsyConfig.OriginalContext == context { - devsyConfig.OriginalContext = "default" + devsyConfig.OriginalContext = config.DefaultContext } } diff --git a/cmd/context/set_options.go b/cmd/context/set_options.go index 18e8bf3e0..cc0ef856e 100644 --- a/cmd/context/set_options.go +++ b/cmd/context/set_options.go @@ -50,30 +50,21 @@ func NewSetOptionsCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *SetOptionsCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig("", cmd.Provider) - if err != nil { - return err - } - - // check for context - if context == "" { - context = devsyConfig.DefaultContext - } else if devsyConfig.Contexts[context] == nil { - return fmt.Errorf("context %q doesn't exist", context) - } - - // check if there are setOptions options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, context, cmd.Options) - if err != nil { - return err + return config.UpdateConfig("", cmd.Provider, func(devsyConfig *config.Config) error { + // check for context + if context == "" { + context = devsyConfig.DefaultContext + } else if devsyConfig.Contexts[context] == nil { + return fmt.Errorf("context %q doesn't exist", context) } - } - err = config.SaveConfig(devsyConfig) - if err != nil { - return fmt.Errorf("save config: %w", err) - } + // check if there are setOptions options set + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, context, cmd.Options); err != nil { + return err + } + } - return nil + return nil + }) } diff --git a/cmd/context/use.go b/cmd/context/use.go index 19971c71b..aaef25ba5 100644 --- a/cmd/context/use.go +++ b/cmd/context/use.go @@ -45,26 +45,19 @@ func NewUseCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *UseCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig("", cmd.Provider) - if err != nil { - return err - } else if devsyConfig.Contexts[context] == nil { - return fmt.Errorf("context %q doesn't exist", context) - } - - // check if there are use options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, context, cmd.Options) - if err != nil { - return err + return config.UpdateConfig("", cmd.Provider, func(devsyConfig *config.Config) error { + if devsyConfig.Contexts[context] == nil { + return fmt.Errorf("context %q doesn't exist", context) } - } - devsyConfig.DefaultContext = context - err = config.SaveConfig(devsyConfig) - if err != nil { - return fmt.Errorf("save config: %w", err) - } + // check if there are use options set + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, context, cmd.Options); err != nil { + return err + } + } - return nil + devsyConfig.DefaultContext = context + return nil + }) } diff --git a/cmd/ide/set.go b/cmd/ide/set.go index b9e563f77..7d1ed02aa 100644 --- a/cmd/ide/set.go +++ b/cmd/ide/set.go @@ -55,23 +55,13 @@ with 'devsy ide list'.`, // Run runs the command logic. func (cmd *SetCmd) Run(_ context.Context, ideName string) error { - devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) - if err != nil { - return err - } - ideName = strings.ToLower(ideName) ideOptions, err := ideparse.GetIDEOptions(ideName) if err != nil { return err } - if err := setOptions(devsyConfig, ideName, cmd.Options, ideOptions); err != nil { - return err - } - - if err := config.SaveConfig(devsyConfig); err != nil { - return fmt.Errorf("save config: %w", err) - } - return nil + return config.UpdateConfig(cmd.Context, cmd.Provider, func(devsyConfig *config.Config) error { + return setOptions(devsyConfig, ideName, cmd.Options, ideOptions) + }) } diff --git a/cmd/ide/use.go b/cmd/ide/use.go index 01d261b89..ccd5a47b6 100644 --- a/cmd/ide/use.go +++ b/cmd/ide/use.go @@ -2,7 +2,6 @@ package ide import ( "context" - "fmt" "maps" "strings" @@ -52,29 +51,25 @@ Available IDEs can be listed with 'devsy ide list'`, // Run runs the command logic. func (cmd *UseCmd) Run(ctx context.Context, ide string) error { - devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) - if err != nil { - return err - } - ide = strings.ToLower(ide) ideOptions, err := ideparse.GetIDEOptions(ide) if err != nil { return err } - // check if there are user options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, ide, cmd.Options, ideOptions) - if err != nil { - return err + err = config.UpdateConfig(cmd.Context, cmd.Provider, func(devsyConfig *config.Config) error { + // check if there are user options set + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, ide, cmd.Options, ideOptions); err != nil { + return err + } } - } - devsyConfig.Current().DefaultIDE = ide - err = config.SaveConfig(devsyConfig) + devsyConfig.Current().DefaultIDE = ide + return nil + }) if err != nil { - return fmt.Errorf("save config: %w", err) + return err } log.Infof("default IDE set to %q", ide) diff --git a/cmd/mcp/tools_provider.go b/cmd/mcp/tools_provider.go index 363b533f2..9198bf93b 100644 --- a/cmd/mcp/tools_provider.go +++ b/cmd/mcp/tools_provider.go @@ -142,6 +142,11 @@ func runProviderAdd(ctx context.Context, g *flags.GlobalFlags, in providerAddInp } func runProviderDelete(ctx context.Context, g *flags.GlobalFlags, name string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() devsyConfig, err := config.LoadConfig(g.Context, g.Provider) if err != nil { return err @@ -150,9 +155,5 @@ func runProviderDelete(ctx context.Context, g *flags.GlobalFlags, name string) e } func runProviderUse(_ context.Context, g *flags.GlobalFlags, name string) error { - devsyConfig, err := config.LoadConfig(g.Context, g.Provider) - if err != nil { - return err - } - return cmdprovider.UseProvider(devsyConfig, name) + return cmdprovider.UseProvider(g.Context, g.Provider, name) } diff --git a/cmd/pro/login.go b/cmd/pro/login.go index b176d518c..3d11b8169 100644 --- a/cmd/pro/login.go +++ b/cmd/pro/login.go @@ -108,6 +108,12 @@ func (cmd *LoginCmd) Run(ctx context.Context, fullURL string) error { return err } + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, currentInstance, err := cmd.resolveInstance(fullURL) if err != nil { return err diff --git a/cmd/pro/logout.go b/cmd/pro/logout.go index c9dfd6f05..505e8a35c 100644 --- a/cmd/pro/logout.go +++ b/cmd/pro/logout.go @@ -118,8 +118,17 @@ func (cmd *LogoutCmd) Run(ctx context.Context, args []string) error { } } - // delete the provider config - err = providercmd.DeleteProviderConfig(devsyConfig, proInstanceConfig.Provider, true) + // delete the provider config, reloading it under the config lock: the + // earlier load in this flow predates the daemon shutdown above + unlock, err := config.LockConfig() + if err != nil { + return err + } + freshConfig, err := config.LoadConfig(devsyConfig.DefaultContext, "") + if err == nil { + err = providercmd.DeleteProviderConfig(freshConfig, proInstanceConfig.Provider, true) + } + unlock() if err != nil { return err } diff --git a/cmd/pro/update_provider.go b/cmd/pro/update_provider.go index e7a77e5d9..869458948 100644 --- a/cmd/pro/update_provider.go +++ b/cmd/pro/update_provider.go @@ -49,6 +49,12 @@ func (cmd *UpdateProviderCmd) Run(ctx context.Context, args []string) error { } newVersion := args[0] + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/add.go b/cmd/provider/add.go index 11d15105a..5d1e48555 100644 --- a/cmd/provider/add.go +++ b/cmd/provider/add.go @@ -48,6 +48,12 @@ func NewAddCmd(f *flags.GlobalFlags) *cobra.Command { }, RunE: func(cobraCmd *cobra.Command, args []string) error { ctx := cobraCmd.Context() + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/delete.go b/cmd/provider/delete.go index 8323c3af9..4e9da1068 100644 --- a/cmd/provider/delete.go +++ b/cmd/provider/delete.go @@ -61,6 +61,12 @@ func NewDeleteCmd(flags *flags.GlobalFlags) *cobra.Command { } func (cmd *DeleteCmd) Run(ctx context.Context, args []string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/init.go b/cmd/provider/init.go index d7c33c83e..731df238d 100644 --- a/cmd/provider/init.go +++ b/cmd/provider/init.go @@ -29,6 +29,12 @@ func NewInitCmd(f *flags.GlobalFlags) *cobra.Command { Short: "Run or re-run init and option resolution for an existing provider", Args: cobra.MaximumNArgs(1), RunE: func(cobraCmd *cobra.Command, args []string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/rename.go b/cmd/provider/rename.go index b8b867df6..58235aee8 100644 --- a/cmd/provider/rename.go +++ b/cmd/provider/rename.go @@ -48,6 +48,14 @@ func (cmd *RenameCmd) Run(ctx context.Context, args []string) error { return err } + // The rename rewrites provider, workspace, and machine state across several + // config saves with rollback, so the whole transaction runs under the lock. + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/set.go b/cmd/provider/set.go index d658b7051..6eb67dc13 100644 --- a/cmd/provider/set.go +++ b/cmd/provider/set.go @@ -63,6 +63,12 @@ func NewSetCmd(f *flags.GlobalFlags) *cobra.Command { } func (cmd *SetCmd) Run(ctx context.Context, args []string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, providerWithOptions, err := cmd.loadProvider(args) if err != nil { return err diff --git a/cmd/provider/set_source.go b/cmd/provider/set_source.go index 7b9dbc140..e1caa348b 100644 --- a/cmd/provider/set_source.go +++ b/cmd/provider/set_source.go @@ -35,6 +35,12 @@ func NewSetSourceCmd(flags *flags.GlobalFlags) *cobra.Command { Short: "Set or change a provider's source (replaces the registered name, repo, URL, or path)", RunE: func(cobraCmd *cobra.Command, args []string) error { ctx := cobraCmd.Context() + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/use.go b/cmd/provider/use.go index 67903547b..19e38e045 100644 --- a/cmd/provider/use.go +++ b/cmd/provider/use.go @@ -1,8 +1,6 @@ package provider import ( - "fmt" - "github.com/devsy-org/devsy/cmd/completion" "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/config" @@ -11,17 +9,27 @@ import ( "github.com/spf13/cobra" ) -// UseProvider sets the named provider as the default for the active config context. -func UseProvider(devsyConfig *config.Config, name string) error { - p, err := workspace.FindProvider(devsyConfig, name) +// UseProvider sets the named provider as the default for the addressed config +// context, loading and saving config.yaml under the config lock. +func UseProvider(contextOverride, providerOverride, name string) error { + var resolved string + err := config.UpdateConfig( + contextOverride, + providerOverride, + func(devsyConfig *config.Config) error { + p, err := workspace.FindProvider(devsyConfig, name) + if err != nil { + return err + } + devsyConfig.Current().DefaultProvider = p.Config.Name + resolved = p.Config.Name + return nil + }, + ) if err != nil { return err } - devsyConfig.Current().DefaultProvider = p.Config.Name - if err := config.SaveConfig(devsyConfig); err != nil { - return fmt.Errorf("save config: %w", err) - } - log.Infof("default provider: %s", p.Config.Name) + log.Infof("default provider: %s", resolved) return nil } @@ -38,20 +46,7 @@ func NewUseCmd(f *flags.GlobalFlags) *cobra.Command { Short: "Set the default provider for the active context", Args: cobra.ExactArgs(1), RunE: func(cobraCmd *cobra.Command, args []string) error { - devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) - if err != nil { - return err - } - p, err := workspace.FindProvider(devsyConfig, args[0]) - if err != nil { - return err - } - devsyConfig.Current().DefaultProvider = p.Config.Name - if err := config.SaveConfig(devsyConfig); err != nil { - return fmt.Errorf("save config: %w", err) - } - log.Infof("default provider: %s", p.Config.Name) - return nil + return UseProvider(cmd.Context, cmd.Provider, args[0]) }, ValidArgsFunction: func( rootCmd *cobra.Command, diff --git a/cmd/workspace/import.go b/cmd/workspace/import.go index cd3c7f7bb..bce4b5b2d 100644 --- a/cmd/workspace/import.go +++ b/cmd/workspace/import.go @@ -109,6 +109,12 @@ func (cmd *ImportCmd) execute(ctx context.Context) error { if parseErr != nil { return parseErr } + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/pkg/config/config.go b/pkg/config/config.go index 1798f5268..fb33e9cf4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -358,6 +358,25 @@ func SaveConfig(config *Config) error { return writeConfigAtomic(configOrigin, out) } +// UpdateConfig runs a read/modify/write cycle on config.yaml under LockConfig: +// the config is loaded after the lock is acquired and saved before it is +// released, so concurrent mutations cannot lose each other's changes. +func UpdateConfig(contextOverride, providerOverride string, mutate func(*Config) error) error { + unlock, err := LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := LoadConfig(contextOverride, providerOverride) + if err != nil { + return err + } + if err := mutate(devsyConfig); err != nil { + return err + } + return SaveConfig(devsyConfig) +} + // LockConfig serializes read/modify/write mutations to config.yaml across // processes. Callers must acquire it before loading the config and hold it // until every related persistent operation is complete. diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go index 429be2895..48b62cd3b 100644 --- a/pkg/workspace/provider_update.go +++ b/pkg/workspace/provider_update.go @@ -97,6 +97,18 @@ func applyProviderUpdate( } providerSource = splitted[0] + "@" + newVersion + // The caller's config predates the update check; reload it under the + // config lock so the update cannot lose a concurrent mutation. + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err = config.LoadConfig(devsyConfig.DefaultContext, "") + if err != nil { + return err + } + _, err = UpdateProvider(ctx, devsyConfig, providerName, providerSource) if err != nil { return fmt.Errorf("update provider %s: %w", providerName, err) From 955ff57be85b766b815ba5a4d80f36af46759499 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 24 Sep 2026 23:49:15 -0600 Subject: [PATCH 03/10] chore: remove redundant implementation comments --- cmd/context/create.go | 2 -- cmd/context/set_options.go | 2 -- cmd/context/use.go | 1 - cmd/ide/use.go | 1 - desktop/src/main/ipc.ts | 3 +-- 5 files changed, 1 insertion(+), 8 deletions(-) diff --git a/cmd/context/create.go b/cmd/context/create.go index 0a36ba6e2..3daa79f8b 100644 --- a/cmd/context/create.go +++ b/cmd/context/create.go @@ -51,7 +51,6 @@ func (cmd *CreateCmd) Run(ctx context.Context, context string) error { return fmt.Errorf("context %q already exists", context) } - // verify name if provider2.ProviderNameRegEx.MatchString(context) { return fmt.Errorf("context name can only include lower case letters, numbers or dashes") } else if len(context) > 48 { @@ -59,7 +58,6 @@ func (cmd *CreateCmd) Run(ctx context.Context, context string) error { } devsyConfig.Contexts[context] = &config.ContextConfig{} - // check if there are create options set if len(cmd.Options) > 0 { if err := setOptions(devsyConfig, context, cmd.Options); err != nil { return err diff --git a/cmd/context/set_options.go b/cmd/context/set_options.go index cc0ef856e..c94040b30 100644 --- a/cmd/context/set_options.go +++ b/cmd/context/set_options.go @@ -51,14 +51,12 @@ func NewSetOptionsCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *SetOptionsCmd) Run(ctx context.Context, context string) error { return config.UpdateConfig("", cmd.Provider, func(devsyConfig *config.Config) error { - // check for context if context == "" { context = devsyConfig.DefaultContext } else if devsyConfig.Contexts[context] == nil { return fmt.Errorf("context %q doesn't exist", context) } - // check if there are setOptions options set if len(cmd.Options) > 0 { if err := setOptions(devsyConfig, context, cmd.Options); err != nil { return err diff --git a/cmd/context/use.go b/cmd/context/use.go index aaef25ba5..33af291b3 100644 --- a/cmd/context/use.go +++ b/cmd/context/use.go @@ -50,7 +50,6 @@ func (cmd *UseCmd) Run(ctx context.Context, context string) error { return fmt.Errorf("context %q doesn't exist", context) } - // check if there are use options set if len(cmd.Options) > 0 { if err := setOptions(devsyConfig, context, cmd.Options); err != nil { return err diff --git a/cmd/ide/use.go b/cmd/ide/use.go index ccd5a47b6..f83b8c76c 100644 --- a/cmd/ide/use.go +++ b/cmd/ide/use.go @@ -58,7 +58,6 @@ func (cmd *UseCmd) Run(ctx context.Context, ide string) error { } err = config.UpdateConfig(cmd.Context, cmd.Provider, func(devsyConfig *config.Config) error { - // check if there are user options set if len(cmd.Options) > 0 { if err := setOptions(devsyConfig, ide, cmd.Options, ideOptions); err != nil { return err diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index f26acea7b..c65b31d50 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -1591,8 +1591,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, wsId, ) - // Expose a method to suppress callbacks from cancelActiveUp - ;( + ;( child as unknown as { _suppressWorkspaceCallbacks?: () => void } )._suppressWorkspaceCallbacks = () => { suppressCallbacks = true From b5218ac0526d3b92a87f82ce166ea91087ee4002 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 25 Sep 2026 00:07:56 -0600 Subject: [PATCH 04/10] test(desktop): allow workspace deletion reconciliation --- desktop/e2e/workspaces.e2e.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/e2e/workspaces.e2e.ts b/desktop/e2e/workspaces.e2e.ts index 6220ffde2..47cee6408 100644 --- a/desktop/e2e/workspaces.e2e.ts +++ b/desktop/e2e/workspaces.e2e.ts @@ -99,7 +99,7 @@ test.describe("Workspace lifecycle badges", () => { await expect(main).toContainText("Deleting", { timeout: 3000 }) await expect(main.locator("text=deleteprobe")).not.toBeVisible({ - timeout: 5000, + timeout: 10000, }) await waitForDeleteToSettle() } catch (error) { From 7555e38a2261767c0f810136d446210ff9c1f950 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 25 Sep 2026 01:21:38 -0600 Subject: [PATCH 05/10] test(e2e): avoid machine provider startup timeout race --- e2e/tests/machineprovider/machineprovider.go | 8 +++----- .../testdata/machineprovider3/provider.yaml | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/e2e/tests/machineprovider/machineprovider.go b/e2e/tests/machineprovider/machineprovider.go index b57288245..3ba6a2cdf 100644 --- a/e2e/tests/machineprovider/machineprovider.go +++ b/e2e/tests/machineprovider/machineprovider.go @@ -182,7 +182,7 @@ var _ = ginkgo.Describe( framework.ExpectNoError(err) ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) - // create provider (same 5s inactivity timeout as machineprovider2) + // create provider with a short timeout while leaving startup headroom _ = f.DevsyProviderDelete(ctx, "docker123") err = f.DevsyProviderAdd(ctx, filepath.Join(tempDir, "provider.yaml")) framework.ExpectNoError(err) @@ -213,14 +213,12 @@ var _ = ginkgo.Describe( err = f.DevsyUp(ctx, tempDir, "--daemon-interval=3s") framework.ExpectNoError(err) - // verify workspace stays running well past the 5s timeout. - // The timeout would fire within ~15s (5s timeout + 10s ticker). - // We assert RUNNING for 30s to give ample margin. + // Verify workspace stays running past the 30s timeout and its 10s ticker. gomega.Consistently(func() string { status, err := f.DevsyStatus(ctx, tempDir, "--container-status=false") framework.ExpectNoError(err) return strings.ToUpper(status.State) - }, 30*time.Second, 2*time.Second).Should( + }, 50*time.Second, 2*time.Second).Should( gomega.Equal("RUNNING"), "workspace should stay running when shutdownAction is none", ) diff --git a/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml b/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml index 1b14a88b7..2e0c50805 100644 --- a/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml +++ b/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml @@ -8,7 +8,7 @@ options: default: devsy-e2e INACTIVITY_TIMEOUT: description: The timeout until the pod will be stopped - default: 5s + default: 30s agent: path: /usr/local/bin/devsy inactivityTimeout: ${INACTIVITY_TIMEOUT} From b42c95d98f677874273c13a6fc81602d3458df95 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 25 Sep 2026 02:36:42 -0700 Subject: [PATCH 06/10] fix: address greptile review findings - pro login: hold the config lock only for the config changes, not the interactive browser wait - provider auto-update: re-check the version after reloading under the lock so a concurrent newer update is never replaced - Secrets page: remount the attach switch on failure so a failed toggle cannot display the wrong state - secrets list tests: align with the testify/suite unit-test convention --- cmd/pro/login.go | 28 ++++++++++----- cmd/secrets/list_test.go | 34 ++++++++++++------- .../src/renderer/src/pages/SecretsPage.svelte | 4 +++ pkg/workspace/provider_update.go | 17 ++++++++++ 4 files changed, 62 insertions(+), 21 deletions(-) diff --git a/cmd/pro/login.go b/cmd/pro/login.go index 3d11b8169..4fdd9d05b 100644 --- a/cmd/pro/login.go +++ b/cmd/pro/login.go @@ -108,23 +108,29 @@ func (cmd *LoginCmd) Run(ctx context.Context, fullURL string) error { return err } - unlock, err := config.LockConfig() + devsyConfig, err := cmd.prepareProvider(ctx, fullURL) if err != nil { return err } - defer unlock() - devsyConfig, currentInstance, err := cmd.resolveInstance(fullURL) + return cmd.loginAndConfigure(ctx, devsyConfig, fullURL) +} + +// prepareProvider applies the login-related config changes under the config +// lock; the interactive browser login itself runs unlocked. +func (cmd *LoginCmd) prepareProvider(ctx context.Context, fullURL string) (*config.Config, error) { + unlock, err := config.LockConfig() if err != nil { - return err + return nil, err } + defer unlock() - devsyConfig, err = cmd.ensureProvider(ctx, devsyConfig, currentInstance, fullURL) + devsyConfig, currentInstance, err := cmd.resolveInstance(fullURL) if err != nil { - return err + return nil, err } - return cmd.loginAndConfigure(ctx, devsyConfig, fullURL) + return cmd.ensureProvider(ctx, devsyConfig, currentInstance, fullURL) } func (cmd *LoginCmd) normalizeURL(fullURL string) (string, error) { @@ -285,8 +291,14 @@ func (cmd *LoginCmd) loginAndConfigure( } if cmd.Use { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + // Post-login: preserve user values; resolver prunes anything stale. - err := providercmd.ConfigureProvider(ctx, providercmd.ProviderOptionsConfig{ + err = providercmd.ConfigureProvider(ctx, providercmd.ProviderOptionsConfig{ Provider: providerConfig, ContextName: devsyConfig.DefaultContext, UserOptions: cmd.Options, diff --git a/cmd/secrets/list_test.go b/cmd/secrets/list_test.go index 713d6eef2..266e0f4dc 100644 --- a/cmd/secrets/list_test.go +++ b/cmd/secrets/list_test.go @@ -6,9 +6,17 @@ import ( "github.com/devsy-org/devsy/pkg/config" devsysecrets "github.com/devsy-org/devsy/pkg/secrets" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" ) +type ListTestSuite struct { + suite.Suite +} + +func TestListSuite(t *testing.T) { + suite.Run(t, new(ListTestSuite)) +} + type listTestStore struct { metas []devsysecrets.SecretMeta } @@ -21,7 +29,7 @@ func (s *listTestStore) Meta(string, string) (devsysecrets.SecretMeta, error) { func (s *listTestStore) List(string) ([]devsysecrets.SecretMeta, error) { return s.metas, nil } func (s *listTestStore) Delete(string, string) error { return nil } -func TestListEntriesMarksAttachedSecrets(t *testing.T) { +func (s *ListTestSuite) TestListEntriesMarksAttachedSecrets() { created := time.Date(2026, 9, 24, 12, 0, 0, 0, time.UTC) store := &listTestStore{metas: []devsysecrets.SecretMeta{ { @@ -36,23 +44,23 @@ func TestListEntriesMarksAttachedSecrets(t *testing.T) { cfg := deleteTestConfig([]string{"ATTACHED", "sops:project/API_TOKEN"}) entries, err := listEntries(cfg, store, config.DefaultContext) - require.NoError(t, err) - require.Len(t, entries, 2) - require.Equal(t, "ATTACHED", entries[0].Name) - require.True(t, entries[0].Attached) - require.Equal(t, created.Format(time.RFC3339), entries[0].Created) - require.Equal(t, "DETACHED", entries[1].Name) - require.False(t, entries[1].Attached) + s.Require().NoError(err) + s.Require().Len(entries, 2) + s.Require().Equal("ATTACHED", entries[0].Name) + s.Require().True(entries[0].Attached) + s.Require().Equal(created.Format(time.RFC3339), entries[0].Created) + s.Require().Equal("DETACHED", entries[1].Name) + s.Require().False(entries[1].Attached) } -func TestListEntriesDetachedWhenContextHasNoBindings(t *testing.T) { +func (s *ListTestSuite) TestListEntriesDetachedWhenContextHasNoBindings() { store := &listTestStore{metas: []devsysecrets.SecretMeta{ {Name: "TOKEN", Context: config.DefaultContext, Kind: devsysecrets.KindSecret}, }} cfg := deleteTestConfig(nil) entries, err := listEntries(cfg, store, config.DefaultContext) - require.NoError(t, err) - require.Len(t, entries, 1) - require.False(t, entries[0].Attached) + s.Require().NoError(err) + s.Require().Len(entries, 1) + s.Require().False(entries[0].Attached) } diff --git a/desktop/src/renderer/src/pages/SecretsPage.svelte b/desktop/src/renderer/src/pages/SecretsPage.svelte index de1cc570e..15d8666c1 100644 --- a/desktop/src/renderer/src/pages/SecretsPage.svelte +++ b/desktop/src/renderer/src/pages/SecretsPage.svelte @@ -35,6 +35,7 @@ let confirmDeleteOpen = $state(false) let pendingDelete = $state("") let deleting = $state(false) let updatingAttachment = $state>({}) +let attachmentResets = $state>({}) let attachmentErrors = $state>({}) let searchTerm = $state("") @@ -88,6 +89,7 @@ async function setAttached(secret: Secret, attached: boolean) { await refreshSecrets() } catch (err) { attachmentErrors = { ...attachmentErrors, [key]: extractErrorMessage(err) } + attachmentResets = { ...attachmentResets, [key]: (attachmentResets[key] ?? 0) + 1 } } finally { updatingAttachment = { ...updatingAttachment, [key]: false } } @@ -235,12 +237,14 @@ async function confirmDelete() {

Inject into workspaces

Context: {secret.context}. Delivered through the protected secret path when a workspace starts or is recreated.

+ {#key attachmentResets[`${secret.context}\x00${secret.name}`] ?? 0} setAttached(secret, checked)} /> + {/key} {#if attachmentErrors[`${secret.context}\x00${secret.name}`]}

diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go index 48b62cd3b..cf18f0ef0 100644 --- a/pkg/workspace/provider_update.go +++ b/pkg/workspace/provider_update.go @@ -109,6 +109,23 @@ func applyProviderUpdate( return err } + // Another command may have updated the provider while this one waited on + // the lock; never replace an equal or newer version with this one. + currentSource, err := ResolveProviderSource(devsyConfig, providerName) + if err != nil { + return fmt.Errorf("resolve provider source %s: %w", providerName, err) + } + currentVersion := "" + if parts := strings.Split(currentSource, "@"); len(parts) == 2 { + currentVersion = parts[1] + } + newV, newErr := semver.Parse(strings.TrimPrefix(newVersion, "v")) + currentV, currentErr := semver.Parse(strings.TrimPrefix(currentVersion, "v")) + if newErr == nil && currentErr == nil && currentV.GTE(newV) { + log.Infof("provider already up to date, skipping update: provider=%s", providerName) + return nil + } + _, err = UpdateProvider(ctx, devsyConfig, providerName, providerSource) if err != nil { return fmt.Errorf("update provider %s: %w", providerName, err) From c2c318a07d609cb66c663cf688ab7ad69b28c78f Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 25 Sep 2026 13:42:57 -0700 Subject: [PATCH 07/10] fix(config): skip auto-update when the provider source changed under the lock --- pkg/workspace/provider_update.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go index cf18f0ef0..90f925869 100644 --- a/pkg/workspace/provider_update.go +++ b/pkg/workspace/provider_update.go @@ -110,11 +110,16 @@ func applyProviderUpdate( } // Another command may have updated the provider while this one waited on - // the lock; never replace an equal or newer version with this one. + // the lock; never replace an equal or newer version with this one, and + // never undo a source change made in the meantime. currentSource, err := ResolveProviderSource(devsyConfig, providerName) if err != nil { return fmt.Errorf("resolve provider source %s: %w", providerName, err) } + if strings.Split(currentSource, "@")[0] != splitted[0] { + log.Infof("provider source changed, skipping update: provider=%s", providerName) + return nil + } currentVersion := "" if parts := strings.Split(currentSource, "@"); len(parts) == 2 { currentVersion = parts[1] From 887149c3bf3779648d243e8e0a660af6398c89a0 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Fri, 25 Sep 2026 15:12:22 -0600 Subject: [PATCH 08/10] fix(workspace): preserve concurrent provider source changes --- pkg/workspace/provider_update.go | 34 +++++++++-------- pkg/workspace/provider_update_test.go | 54 +++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go index 90f925869..dc28cac2e 100644 --- a/pkg/workspace/provider_update.go +++ b/pkg/workspace/provider_update.go @@ -91,11 +91,12 @@ func applyProviderUpdate( return fmt.Errorf("resolve provider source %s: %w", providerName, err) } - splitted := strings.Split(providerSource, "@") - if len(splitted) == 0 { + originalSource := providerSource + sourceBase, _ := provider2.SplitSourceAndTag(providerSource) + if sourceBase == "" { return fmt.Errorf("no provider source found %s", providerSource) } - providerSource = splitted[0] + "@" + newVersion + providerSource = sourceBase + "@" + newVersion // The caller's config predates the update check; reload it under the // config lock so the update cannot lose a concurrent mutation. @@ -116,18 +117,8 @@ func applyProviderUpdate( if err != nil { return fmt.Errorf("resolve provider source %s: %w", providerName, err) } - if strings.Split(currentSource, "@")[0] != splitted[0] { - log.Infof("provider source changed, skipping update: provider=%s", providerName) - return nil - } - currentVersion := "" - if parts := strings.Split(currentSource, "@"); len(parts) == 2 { - currentVersion = parts[1] - } - newV, newErr := semver.Parse(strings.TrimPrefix(newVersion, "v")) - currentV, currentErr := semver.Parse(strings.TrimPrefix(currentVersion, "v")) - if newErr == nil && currentErr == nil && currentV.GTE(newV) { - log.Infof("provider already up to date, skipping update: provider=%s", providerName) + if reason := providerUpdateSkipReason(originalSource, currentSource, newVersion); reason != "" { + log.Infof("%s, skipping update: provider=%s", reason, providerName) return nil } @@ -140,6 +131,19 @@ func applyProviderUpdate( return nil } +func providerUpdateSkipReason(originalSource, currentSource, newVersion string) string { + if originalSource != currentSource { + return "provider source changed" + } + _, currentVersion := provider2.SplitSourceAndTag(currentSource) + newV, newErr := semver.Parse(strings.TrimPrefix(newVersion, "v")) + currentV, currentErr := semver.Parse(strings.TrimPrefix(currentVersion, "v")) + if newErr == nil && currentErr == nil && currentV.GTE(newV) { + return "provider already up to date" + } + return "" +} + // GetProInstance returns the ProInstance associated with the given provider name, or nil if not found. func GetProInstance( devsyConfig *config.Config, diff --git a/pkg/workspace/provider_update_test.go b/pkg/workspace/provider_update_test.go index e079a9194..4d607bd9a 100644 --- a/pkg/workspace/provider_update_test.go +++ b/pkg/workspace/provider_update_test.go @@ -114,3 +114,57 @@ func TestProviderVersionNeedsUpdate(t *testing.T) { }) } } + +func TestProviderUpdateSkipReason(t *testing.T) { + const ( + originalSource = "github.com/org/provider@v1.0.0" + updatedPin = "github.com/org/provider@v1.1.0" + otherSource = "github.com/other/provider@v1.0.0" + ) + + tests := []struct { + name string + originalSource string + currentSource string + newVersion string + wantReason string + }{ + { + name: "unchanged source", + originalSource: originalSource, + currentSource: originalSource, + newVersion: "v1.1.0", + }, + { + name: "pin changed on same repository", + originalSource: originalSource, + currentSource: updatedPin, + newVersion: "v1.2.0", + wantReason: "provider source changed", + }, + { + name: "repository changed", + originalSource: originalSource, + currentSource: otherSource, + newVersion: "v1.2.0", + wantReason: "provider source changed", + }, + { + name: "current version is newer", + originalSource: updatedPin, + currentSource: updatedPin, + newVersion: "v1.0.0", + wantReason: "provider already up to date", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal( + t, + tt.wantReason, + providerUpdateSkipReason(tt.originalSource, tt.currentSource, tt.newVersion), + ) + }) + } +} From 705201cfa685f048a1885eac33af83e98300ebeb Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 26 Sep 2026 01:01:07 -0700 Subject: [PATCH 09/10] fix(provider): guard pinned source and loaded version under lock --- pkg/workspace/provider_update.go | 25 ++++++++----- pkg/workspace/provider_update_test.go | 52 ++++++++++++++++++--------- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go index dc28cac2e..c88d78064 100644 --- a/pkg/workspace/provider_update.go +++ b/pkg/workspace/provider_update.go @@ -86,12 +86,12 @@ func applyProviderUpdate( devsyConfig *config.Config, providerName, newVersion string, ) error { - providerSource, err := ResolveProviderSource(devsyConfig, providerName) + originalProvider, err := FindProvider(devsyConfig, providerName) if err != nil { - return fmt.Errorf("resolve provider source %s: %w", providerName, err) + return fmt.Errorf("find provider %s: %w", providerName, err) } - - originalSource := providerSource + originalSource := originalProvider.Config.Source + providerSource := provider2.GetProviderSource(originalSource, originalProvider.Config.Name) sourceBase, _ := provider2.SplitSourceAndTag(providerSource) if sourceBase == "" { return fmt.Errorf("no provider source found %s", providerSource) @@ -113,11 +113,16 @@ func applyProviderUpdate( // Another command may have updated the provider while this one waited on // the lock; never replace an equal or newer version with this one, and // never undo a source change made in the meantime. - currentSource, err := ResolveProviderSource(devsyConfig, providerName) + currentProvider, err := FindProvider(devsyConfig, providerName) if err != nil { - return fmt.Errorf("resolve provider source %s: %w", providerName, err) + return fmt.Errorf("find provider %s: %w", providerName, err) } - if reason := providerUpdateSkipReason(originalSource, currentSource, newVersion); reason != "" { + if reason := providerUpdateSkipReason( + originalSource, + currentProvider.Config.Source, + currentProvider.Config.Version, + newVersion, + ); reason != "" { log.Infof("%s, skipping update: provider=%s", reason, providerName) return nil } @@ -131,11 +136,13 @@ func applyProviderUpdate( return nil } -func providerUpdateSkipReason(originalSource, currentSource, newVersion string) string { +func providerUpdateSkipReason( + originalSource, currentSource provider2.ProviderSource, + currentVersion, newVersion string, +) string { if originalSource != currentSource { return "provider source changed" } - _, currentVersion := provider2.SplitSourceAndTag(currentSource) newV, newErr := semver.Parse(strings.TrimPrefix(newVersion, "v")) currentV, currentErr := semver.Parse(strings.TrimPrefix(currentVersion, "v")) if newErr == nil && currentErr == nil && currentV.GTE(newV) { diff --git a/pkg/workspace/provider_update_test.go b/pkg/workspace/provider_update_test.go index 4d607bd9a..186eefccf 100644 --- a/pkg/workspace/provider_update_test.go +++ b/pkg/workspace/provider_update_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/provider" "github.com/stretchr/testify/assert" ) @@ -115,56 +116,73 @@ func TestProviderVersionNeedsUpdate(t *testing.T) { } } +func TestProviderUpdateSkipReasonResolvedGitHubSource(t *testing.T) { + original := provider.ProviderSource{Github: "org/provider", Raw: "org/provider@v1.0.0"} + changed := provider.ProviderSource{Github: "org/provider", Raw: "org/provider@v1.1.0"} + assert.Equal( + t, + provider.GetProviderSource(original, "provider"), + provider.GetProviderSource(changed, "provider"), + ) + assert.Equal(t, "provider source changed", providerUpdateSkipReason( + original, changed, "v1.0.0", "v1.2.0", + )) + assert.Equal(t, "provider already up to date", providerUpdateSkipReason( + original, original, "v1.3.0", "v1.2.0", + )) +} + func TestProviderUpdateSkipReason(t *testing.T) { const ( originalSource = "github.com/org/provider@v1.0.0" updatedPin = "github.com/org/provider@v1.1.0" otherSource = "github.com/other/provider@v1.0.0" ) - tests := []struct { name string - originalSource string - currentSource string + originalSource provider.ProviderSource + currentSource provider.ProviderSource + currentVersion string newVersion string wantReason string }{ { name: "unchanged source", - originalSource: originalSource, - currentSource: originalSource, + originalSource: provider.ProviderSource{Raw: originalSource}, + currentSource: provider.ProviderSource{Raw: originalSource}, + currentVersion: "v1.0.0", newVersion: "v1.1.0", }, { name: "pin changed on same repository", - originalSource: originalSource, - currentSource: updatedPin, + originalSource: provider.ProviderSource{Raw: originalSource}, + currentSource: provider.ProviderSource{Raw: updatedPin}, + currentVersion: "v1.1.0", newVersion: "v1.2.0", wantReason: "provider source changed", }, { name: "repository changed", - originalSource: originalSource, - currentSource: otherSource, + originalSource: provider.ProviderSource{Raw: originalSource}, + currentSource: provider.ProviderSource{Raw: otherSource}, + currentVersion: "v1.0.0", newVersion: "v1.2.0", wantReason: "provider source changed", }, { name: "current version is newer", - originalSource: updatedPin, - currentSource: updatedPin, + originalSource: provider.ProviderSource{Raw: updatedPin}, + currentSource: provider.ProviderSource{Raw: updatedPin}, + currentVersion: "v1.1.0", newVersion: "v1.0.0", wantReason: "provider already up to date", }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal( - t, - tt.wantReason, - providerUpdateSkipReason(tt.originalSource, tt.currentSource, tt.newVersion), - ) + assert.Equal(t, tt.wantReason, providerUpdateSkipReason( + tt.originalSource, tt.currentSource, tt.currentVersion, tt.newVersion, + )) }) } } From 6d01f8ae8906144a4b8cac670af427c39452dafe Mon Sep 17 00:00:00 2001 From: Samuel K Date: Sat, 26 Sep 2026 01:29:02 -0700 Subject: [PATCH 10/10] test(provider): deduplicate version fixtures for lint --- pkg/workspace/provider_update_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/workspace/provider_update_test.go b/pkg/workspace/provider_update_test.go index 186eefccf..a4eecf934 100644 --- a/pkg/workspace/provider_update_test.go +++ b/pkg/workspace/provider_update_test.go @@ -137,6 +137,8 @@ func TestProviderUpdateSkipReason(t *testing.T) { originalSource = "github.com/org/provider@v1.0.0" updatedPin = "github.com/org/provider@v1.1.0" otherSource = "github.com/other/provider@v1.0.0" + version100 = "v1.0.0" + version110 = "v1.1.0" ) tests := []struct { name string @@ -150,14 +152,14 @@ func TestProviderUpdateSkipReason(t *testing.T) { name: "unchanged source", originalSource: provider.ProviderSource{Raw: originalSource}, currentSource: provider.ProviderSource{Raw: originalSource}, - currentVersion: "v1.0.0", - newVersion: "v1.1.0", + currentVersion: version100, + newVersion: version110, }, { name: "pin changed on same repository", originalSource: provider.ProviderSource{Raw: originalSource}, currentSource: provider.ProviderSource{Raw: updatedPin}, - currentVersion: "v1.1.0", + currentVersion: version110, newVersion: "v1.2.0", wantReason: "provider source changed", }, @@ -165,7 +167,7 @@ func TestProviderUpdateSkipReason(t *testing.T) { name: "repository changed", originalSource: provider.ProviderSource{Raw: originalSource}, currentSource: provider.ProviderSource{Raw: otherSource}, - currentVersion: "v1.0.0", + currentVersion: version100, newVersion: "v1.2.0", wantReason: "provider source changed", }, @@ -173,8 +175,8 @@ func TestProviderUpdateSkipReason(t *testing.T) { name: "current version is newer", originalSource: provider.ProviderSource{Raw: updatedPin}, currentSource: provider.ProviderSource{Raw: updatedPin}, - currentVersion: "v1.1.0", - newVersion: "v1.0.0", + currentVersion: version110, + newVersion: version100, wantReason: "provider already up to date", }, }