Skip to content

Fix Wrangler sync env parity - #1690

Closed
simple-agent-manager[bot] wants to merge 1 commit into
mainfrom
sam/wrangler-sync-env-parity
Closed

Fix Wrangler sync env parity#1690
simple-agent-manager[bot] wants to merge 1 commit into
mainfrom
sam/wrangler-sync-env-parity

Conversation

@simple-agent-manager

Copy link
Copy Markdown
Contributor

Summary

  • Centralizes the sync-wrangler-config.ts env mapping in deploy-reusable.yml with a shared wrangler_sync_env mapping.
  • Makes the first-deploy tail-consumer re-sync inherit the exact same env inputs as the initial sync, including CF_CONTAINER_ENABLED, all consumed CF_CONTAINER_*, SANDBOX_*, and guided setup tunables.
  • Adds regression coverage that derives the optional Worker var list from sync-wrangler-config.ts and checks direct sync/config env mappings, so future env drift fails in tests.
  • Adds a process note in .claude/rules/07-env-and-urls.md to prevent per-step sync env drift.

Validation

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • Additional validation run (if applicable)
  • N/A: no sweep/cron/alarm candidate selection changes.

Additional validation:

  • pnpm vitest run --config scripts/quality/vitest.config.ts scripts/quality/deploy-reusable-workflow.test.ts — passed, 11 tests
  • pnpm exec eslint scripts/quality/deploy-reusable-workflow.test.ts scripts/deploy/sync-wrangler-config.ts — passed
  • pnpm quality:wrangler-bindings — passed
  • npx tsx --check scripts/deploy/sync-wrangler-config.ts — passed
  • pnpm build — passed
  • pnpm --filter @simple-agent-manager/providers exec vitest run tests/unit/vultr-lifecycle.test.ts --testNamePattern "warns and returns empty IP" — passed after the full parallel run hit this unrelated flaky failure once
  • pnpm --filter @simple-agent-manager/ui exec vitest run tests/EmptyState.test.tsx — passed after the full parallel run hit an unrelated worker timeout
  • pnpm --filter @simple-agent-manager/acp-client exec vitest run tests/unit/components/VoiceButton.test.tsx — passed after the full parallel run hit an unrelated worker timeout

pnpm test note: full parallel pnpm test did not complete green locally because of unrelated/flaky full-suite behavior: first run failed one Vultr provider assertion that passed in isolation and on rerun; second run passed providers but timed out in unrelated UI/acp-client first tests after ~8 minutes. The changed workflow quality test passed repeatedly.

Staging Verification (REQUIRED for all code changes — merge-blocking)

All checkboxes below are mandatory for any PR that changes runtime code (.ts, .tsx, .go, etc.). Write N/A: docs-only ONLY if the PR contains zero runtime code changes. See .claude/rules/13-staging-verification.md.

  • Staging deployment green — N/A: workflow/config quality change only; no runtime Worker/web/VM behavior changed.
  • Live app verified via Playwright — N/A: no app/UI runtime behavior changed.
  • Existing workflows confirmed working — N/A locally; PR CI is the verification target for this workflow-quality remediation.
  • New feature/fix verified on staging — N/A: covered by workflow contract tests.
  • Infrastructure verification completed — N/A: no cloud-init, VM agent, DNS, TLS, or VM provisioning code changed.
  • Mobile and desktop verification notes added for UI changes — N/A: no UI changes.

Staging Verification Evidence

N/A: deploy workflow/config quality change only. This PR does not change application runtime behavior. Verification is by static workflow contract tests plus GitHub CI.

UI Compliance Checklist (Required for UI changes)

  • Mobile-first layout verified — N/A
  • Accessibility checks completed — N/A
  • Shared UI components used or exception documented — N/A
  • Playwright visual audit run locally — N/A

End-to-End Verification (Required for multi-component changes)

  • Data flow traced from user input to final outcome with code path citations (see below)
  • Capability test exercises the complete happy path across system boundaries
  • All spec/doc assumptions about existing behavior verified against code
  • If any gap exists between automated test coverage and full E2E, manual verification steps documented below

Data Flow Trace

  1. .github/workflows/deploy-reusable.yml initial sync defines env: &wrangler_sync_env and maps deployment identity, direct sync envs, cf-container vars, sandbox vars, and setup tunables.
  2. .github/workflows/deploy-reusable.yml first-deploy re-sync uses <<: *wrangler_sync_env, so it cannot drift from the initial sync mapping.
  3. scripts/deploy/sync-wrangler-config.ts:getApiWorkerVars() reads optional Worker vars via getOptionalProcessEnvVars([...]); the quality test derives this list and asserts it is represented in the shared workflow mapping.
  4. scripts/deploy/sync-wrangler-config.ts direct reads (PULUMI_STACK, Cloudflare token aliases, ARTIFACTS_BINDING_ENABLED, SETUP_FORCE) and deployment identity (BASE_DOMAIN, RESOURCE_PREFIX) are covered by explicit test expectations.

Untested Gaps

No live staging deploy was triggered for this workflow/config-only PR. CI will validate the repository checks on the PR branch.

Post-Mortem (Required for bug fix PRs)

What broke

First deploys could run the tail-consumer Wrangler re-sync with fewer env inputs than the initial sync. That could silently drop operator overrides such as CF_CONTAINER_ENABLED=false and allow sync-wrangler-config.ts defaults to reappear.

Root cause

The reusable deploy workflow duplicated env mappings per sync step. The second sync block drifted behind the initial sync block.

Class of bug

Duplicated deployment env mappings across multi-phase deploy workflows.

Why it wasn't caught

Existing tests checked selected variables on the initial sync and did not derive expected coverage from the env vars consumed by sync-wrangler-config.ts.

Process fix included in this PR

  • .claude/rules/07-env-and-urls.md now requires sync-consumed Worker vars to be added to the centralized wrangler_sync_env mapping, not ad hoc per-step env blocks.
  • scripts/quality/deploy-reusable-workflow.test.ts now fails if the shared mapping omits env vars consumed by the sync script or if a sync invocation does not use the shared mapping.

Post-mortem file

tasks/archive/2026-07-29-wrangler-sync-env-parity.md

Specialist Review Evidence (Required for agent-authored PRs)

  • All local reviewers completed and findings addressed before merge
  • If any reviewer did NOT complete: needs-human-review label added and merge deferred to human — N/A, all completed. This PR is explicitly do-not-merge by user instruction.
Reviewer Status Outcome
env-validator PASS Shared mapping covers GitHub vars/secrets consistently; no GH_/GITHUB_ prefix misuse; direct sync env and optional Worker env parity covered.
test-engineer PASS Regression test derives optional Worker env vars from the sync script, checks direct env mappings, and requires all sync invocations to use the shared mapping.
task-completion-validator PASS Research, checklist, diff, and acceptance criteria align; no UI/backend or multi-resource path in scope.

Exceptions (If any)

  • Scope: No staging deployment run locally.
  • Rationale: This PR changes GitHub workflow env wiring, a quality test, an internal agent rule, and a task record. It does not change app runtime code. PR CI is the relevant final gate.
  • Expiration: Before merge, required GitHub CI checks must be green. Do not merge without explicit human authorization.

Agent Preflight (Required)

  • Preflight completed before code changes

Classification

  • external-api-change
  • cross-component-change
  • business-logic-change
  • public-surface-change
  • docs-sync-change
  • security-sensitive-change
  • ui-change
  • infra-change

External References

N/A: no external API/library behavior change. Codebase and workflow evidence only.

Codebase Impact Analysis

Affected components:

  • .github/workflows/deploy-reusable.yml — central sync env mapping and first-deploy re-sync inheritance
  • scripts/quality/deploy-reusable-workflow.test.ts — workflow-to-sync-script contract tests
  • .claude/rules/07-env-and-urls.md — process guardrail
  • tasks/archive/2026-07-29-wrangler-sync-env-parity.md — task/post-mortem record

Documentation & Specs

  • Updated internal agent rule .claude/rules/07-env-and-urls.md.
  • Public docs not updated because no new user-facing configuration variable or behavior was introduced.

Constitution & Risk Check

Checked Principle XI/no hardcoded values: this change removes duplicated env mapping drift by centralizing the workflow mapping; no new runtime defaults or fixed limits were added. Risk is limited to GitHub workflow YAML/env resolution and covered by static workflow tests plus CI.

@codspeed-hq

codspeed-hq Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing sam/wrangler-sync-env-parity (dadb4ed) with main (090b237)

Open in CodSpeed

@sonarqubecloud

Copy link
Copy Markdown

@simple-agent-manager

Copy link
Copy Markdown
Contributor Author

Closing in favor of #1697, which integrates this fix (Wrangler sync env parity) and is being completed and merged tonight per Raphaël's backlog-cleanup authorization (SAM task 01KZ9YVKD8FD6JT15FCWRD7TY4). Branch preserved; reopen if #1697 stalls.

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.

1 participant