Skip to content

fix(dev): don't fail env sync on an empty local Secrets Store - #5331

Merged
RSO merged 1 commit into
mainfrom
fix/env-sync-empty-local-secrets-store
Aug 18, 2026
Merged

fix(dev): don't fail env sync on an empty local Secrets Store#5331
RSO merged 1 commit into
mainfrom
fix/env-sync-empty-local-secrets-store

Conversation

@RSO

@RSO RSO commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Problem

The first pnpm dev:env --missing-secrets-only run in a fresh worktree fails while setting up Cloudflare workers:

Creating secrets store secrets...
Failed to list Secrets Store secrets for services/deploy-infra/builder: ✘ [ERROR] List request returned no secrets.
[ELIFECYCLE] Command failed with exit code 1.

--missing-secrets-only (added in #5256) re-listed the local Secrets Store at apply time via secretExists() and threw on any non-zero wrangler exit. Wrangler's local secrets-store secret list exits 1 with List request returned no secrets. when the store is empty — exactly the state of a fresh worktree, since Secrets Store persistence lives under each worker's per-worktree .wrangler directory. So the sync aborted on the first worker with an empty local store.

Fix

Drop the apply-time listing. It was redundant with the planner, which already:

  • prefetches the same listing (plan.ts loadLocalStoreSecrets), and
  • skips secrets that already exist when source-backed refresh is off, i.e. exactly in --missing-secrets-only/--check mode (plan.ts:833), and
  • deliberately treats a failed listing as "empty" rather than fatal (plan.ts:399, plan.ts:417).

Apply now just creates what the plan selected, so it no longer contradicts the plan. Side effect: one fewer wrangler secrets-store secret list per store per run, in line with what #5256 was after.

Verification

  • tsx dev/local/cli.ts env -y --missing-secrets-only in a fresh worktree: completes, creates the missing secret, exit 0 (previously exit 1).
  • Re-run is a clean no-op (exit 0), confirming the plan-level existence check still short-circuits.
  • pnpm run test:dev-local — 212 pass; output.test.ts updated to assert apply creates the planned secrets concurrently and never issues a list call.
  • pnpm format clean.

`dev:env --missing-secrets-only` re-listed the local Secrets Store at
apply time and treated any non-zero wrangler exit as fatal. Wrangler's
local `secrets-store secret list` exits 1 with "List request returned no
secrets." when the store is empty, so the first env sync in a fresh
worktree aborted on the first worker whose per-worktree `.wrangler`
store had no secrets yet.

The re-check was also redundant: computePlan prefetches the same listing
and already skips secrets that exist when source-backed refresh is off,
and it deliberately tolerates a failed listing. Drop the apply-time
listing so apply just creates what the plan selected, which also removes
one wrangler invocation per store per run.
Comment thread dev/local/env-sync/output.ts
@kilo-code-bot

kilo-code-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
dev/local/env-sync/output.ts 359 Dropping the in-lock existence recheck reopens a TOCTOU race: the plan lists secrets outside the per-store process lock, so a concurrent env-sync sharing the persistence dir can make create fail with "already exists" and abort the sync. Tolerate "already exists" or recheck inside the lock.
Files Reviewed (3 files)
  • dev/local/env-sync/index.ts - 0 issues
  • dev/local/env-sync/output.ts - 1 issue
  • dev/local/env-sync/output.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by kimi-k3 · Input: 70K · Output: 5.1K · Cached: 304.6K

Review guidance: REVIEW.md from base branch main

@RSO
RSO enabled auto-merge (squash) August 18, 2026 10:03
@RSO
RSO disabled auto-merge August 18, 2026 10:03
@RSO
RSO merged commit 3ca9f86 into main Aug 18, 2026
14 checks passed
@RSO
RSO deleted the fix/env-sync-empty-local-secrets-store branch August 18, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants