feat(desktop): attach stored secrets to workspaces by skevetter · Pull Request #1272 · devsy-org/devsy

skevetter · 2026-09-25T03:05:54Z

#1266 covered the environment-variable half of the issue. This PR adds the secrets half and the repo-wide config-write locking fix.

Secrets attach (desktop + CLI surface)

secret_attach / secret_detach desktop IPC handlers driving devsy secret attach|detach, mirroring the env_attach pattern.
Per-context "Inject into workspaces" switches on the Secrets page, with per-row pending/error state.
devsy secret list now reports attached in JSON and plain output, mirroring env list.
E2E proves a context-attached secret reaches the workspace at up without --secret and is gone after detach (docker-managed-secret-attached). Attached secrets keep using the protected secret-delivery path, never --env.
Docs note in the secrets guide: the Desktop Secrets page manages the same attachment state.

Config-write locking

Resolves the CodeRabbit finding from #1266: the env/secret attach paths serialized config mutation under LockConfig, but ~20 other SaveConfig callers did not, so concurrent commands could lose each other's updates.

New config.UpdateConfig(contextOverride, providerOverride, mutate): acquire LockConfig, load, mutate, save.
Migrated onto it: context create/delete/use/set-options, ide set/use, provider use (incl. MCP caller).
Entry-level LockConfig for multi-step flows whose inner saves now run under the held lock: provider add/delete/init/set/set-source, provider rename (whole transaction including rollback), workspace import, pro login/logout (logout reloads under lock after daemon shutdown), pro update-provider, MCP provider delete.
The up-triggered provider auto-update reloads the config under the lock before saving.
No locked path calls another locked path (nested flock would deadlock); this was audited across all call chains.

Verification

go build ./..., go vet, gofmt clean.
go test green: cmd/secrets (new attached-list tests), cmd/env, cmd/context, cmd/provider, cmd/mcp, cmd/workspace, pkg/config, pkg/workspace, pkg/provider.
Desktop: vitest 54/54 (new SecretsPage, commands, IPC tests), svelte-check 0 errors, Biome format applied to touched files.
E2E: new docker test compiles and runs in CI.

Commits are unsigned pending the signing pass.

netlify · 2026-09-25T03:05:59Z

✅ Deploy Preview for images-devsy-sh canceled.

Name	Link
🔨 Latest commit	`6238c89`
🔍 Latest deploy log	https://app.netlify.com/projects/images-devsy-sh/deploys/6ab640b3c4a24f0008124118

coderabbitai · 2026-09-25T03:06:00Z

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

netlify · 2026-09-25T03:06:03Z

✅ Deploy Preview for devsydev canceled.

Name	Link
🔨 Latest commit	`6238c89`
🔍 Latest deploy log	https://app.netlify.com/projects/devsydev/deploys/6ab640b37129d90008ee880f

github-actions · 2026-09-25T03:07:44Z

⚠️ This PR contains unsigned commits. To get your PR merged, please sign those commits (git rebase --exec 'git commit -S --amend --no-edit -n' @{upstream}) and force push them to this branch (git push --force-with-lease).

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.

Learn how to use 1Password to sign your commits.

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.

… 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.

skevetter · 2026-09-25T09:09:42Z

@greptileai review

greptile-apps · 2026-09-25T09:14:53Z

Confidence Score: 3/5

[High risk] Refactors config loading and saving with new locking mechanism.

The PR should not merge until interactive login releases the config lock promptly and provider auto-update revalidates its decision after acquiring the lock.

Findings

Login holds the config lock ▶
Auto-update uses a stale decision ▶
Failed toggle can look successful ▶
New tests bypass suite convention ▶

Summary

This PR adds context-scoped secret attachment controls to Desktop and secret-list output, tests attached-secret delivery during workspace creation, and moves config writers under a shared lock.

The lock prevents concurrent config writes from overwriting one another, but its scope includes interactive Pro login.
The provider auto-update reloads under the lock without revisiting its earlier version decision.
The new Secrets page exposes attachment controls and per-row errors.

Diagram

sequenceDiagram
  participant UI as Desktop Secrets page
  participant IPC as Main-process IPC
  participant CLI as Devsy CLI
  participant Config as Locked config.yaml
  UI->>IPC: secret_attach / secret_detach
  IPC->>CLI: secret attach / detach --context
  CLI->>Config: lock, update context binding, save
  CLI-->>UI: command result
  UI->>IPC: secret_list
  IPC->>CLI: secret list
  CLI-->>UI: entries with attached state

_{Reviews (1) · Last reviewed commit: "test(e2e): avoid machine provider startu..."}

greptile-apps · 2026-09-25T09:14:57Z

+	unlock, err := config.LockConfig()
+	if err != nil {
+		return err
+	}
+	defer unlock()


Login holds the config lock

pro login now keeps the config lock while waiting for browser authentication. If the user leaves that flow open, unrelated commands such as secret attach and context use block on the same lock until login finishes. Limit the lock to the config changes rather than the interactive wait.

greptile-apps · 2026-09-25T09:14:57Z

+	devsyConfig, err = config.LoadConfig(devsyConfig.DefaultContext, "")
+	if err != nil {
+		return err
+	}
+
 	_, err = UpdateProvider(ctx, devsyConfig, providerName, providerSource)


Auto-update uses a stale decision

The code decides that a provider needs updating before it takes the lock. If another command updates that provider while this operation waits, the reloaded config contains the newer version, but this path still applies its earlier update decision and can replace it. Check the version again after reloading.

Knowledge Base Used: Configuration resolution

greptile-apps · 2026-09-25T09:14:57Z

+  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 }


Failed toggle can look successful

When attach or detach fails, this path shows an error but does not restore the switch's checked state or reload the saved state. The switch can therefore appear to say that a secret will be injected when it will not, or the reverse. Restore the displayed state on failure.

greptile-apps · 2026-09-25T09:14:57Z

+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) {


New tests bypass suite convention

This test and TestListEntriesDetachedWhenContextHasNoBindings are standalone testing.T functions using require assertions. The repository's unit-testing directive prefers testify/suite; please align these new tests with that requirement before merging.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

- 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

github-actions Bot added the size/xl label Sep 25, 2026

skevetter force-pushed the feat/issue-1265-secrets-attach branch 2 times, most recently from 3e78c79 to 8a1a76c Compare September 25, 2026 03:38

skevetter added 2 commits September 24, 2026 23:30

skevetter force-pushed the feat/issue-1265-secrets-attach branch from 8a1a76c to 233064e Compare September 25, 2026 05:31

chore: remove redundant implementation comments

3af3bac

skevetter changed the title ~~feat(desktop): attach stored secrets to workspaces; serialize all config writes~~ feat(desktop): attach stored secrets to workspaces Sep 25, 2026

skevetter added 2 commits September 25, 2026 00:07

test(desktop): allow workspace deletion reconciliation

cd5a248

test(e2e): avoid machine provider startup timeout race

7e10510

greptile-apps Bot reviewed Sep 25, 2026

View reviewed changes

Navigation Menu

feat(desktop): attach stored secrets to workspaces - #1272

feat(desktop): attach stored secrets to workspaces#1272
skevetter wants to merge 6 commits into
mainfrom
feat/issue-1265-secrets-attach

skevetter commented Sep 25, 2026

Uh oh!

netlify Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 25, 2026

Draft PR not reviewed

Uh oh!

netlify Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 25, 2026

Uh oh!

skevetter commented Sep 25, 2026

Uh oh!

greptile-apps Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 25, 2026

Uh oh!

greptile-apps Bot Sep 25, 2026

Uh oh!

greptile-apps Bot Sep 25, 2026

Uh oh!

greptile-apps Bot Sep 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

skevetter commented Sep 25, 2026

Secrets attach (desktop + CLI surface)

Config-write locking

Verification

Uh oh!

netlify Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for images-devsy-sh canceled.

Uh oh!

coderabbitai Bot commented Sep 25, 2026

Draft PR not reviewed

Uh oh!

netlify Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for devsydev canceled.

Uh oh!

github-actions Bot commented Sep 25, 2026

Uh oh!

skevetter commented Sep 25, 2026

Uh oh!

greptile-apps Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 3/5

Findings

Summary

Uh oh!

greptile-apps Bot Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

netlify Bot commented Sep 25, 2026 •

edited

Loading

netlify Bot commented Sep 25, 2026 •

edited

Loading

greptile-apps Bot commented Sep 25, 2026 •

edited

Loading