From 5f06e5644b6375c5c7f0ae3f4c00564d66f7d8d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:33:01 +0000 Subject: [PATCH 01/35] fix: check out task output branch for workspace dispatch --- .../task-runner/workspace-steps.ts | 10 ++--- ...k-runner-workspace-branch-dispatch.test.ts | 41 +++++++++++-------- .../2026-07-29-safe-output-branch-dispatch.md | 26 ++++++++++++ 3 files changed, 56 insertions(+), 21 deletions(-) create mode 100644 tasks/active/2026-07-29-safe-output-branch-dispatch.md diff --git a/apps/api/src/durable-objects/task-runner/workspace-steps.ts b/apps/api/src/durable-objects/task-runner/workspace-steps.ts index 7edbf26456..4ebb43a4c2 100644 --- a/apps/api/src/durable-objects/task-runner/workspace-steps.ts +++ b/apps/api/src/durable-objects/task-runner/workspace-steps.ts @@ -404,15 +404,15 @@ async function createWorkspaceOnVmAgent( id: state.projectId, repoProvider: projectRepo?.repoProvider ?? 'github', }); + const checkoutBranch = state.config.outputBranch || state.config.branch; + const baseBranch = + checkoutBranch === state.config.branch ? state.config.defaultBranch : state.config.branch; const response = await createWorkspaceOnNode(nodeId, rc.env, state.userId, { workspaceId, repository: state.config.repository, - branch: state.config.branch, - baseBranch: - state.config.branch === state.config.outputBranch - ? state.config.defaultBranch - : state.config.branch, + branch: checkoutBranch, + baseBranch, defaultBranch: state.config.defaultBranch || 'main', repoProvider: gitSource.repoProvider, cloneUrl: gitSource.cloneUrl, diff --git a/apps/api/tests/unit/durable-objects/task-runner-workspace-branch-dispatch.test.ts b/apps/api/tests/unit/durable-objects/task-runner-workspace-branch-dispatch.test.ts index 948d6d1620..6f68cfbeb2 100644 --- a/apps/api/tests/unit/durable-objects/task-runner-workspace-branch-dispatch.test.ts +++ b/apps/api/tests/unit/durable-objects/task-runner-workspace-branch-dispatch.test.ts @@ -89,22 +89,31 @@ describe('TaskRunner workspace branch dispatch', () => { }); it.each([ - [ - 'generated output branch', - 'sam/generated-output-abc123', - 'sam/generated-output-abc123', - 'main', - ], - [ - 'explicit continuation branch', - 'feature/existing-work', - 'sam/continuation-output-def456', - 'feature/existing-work', - ], + { + scenario: 'generated output branch', + configuredBranch: 'sam/generated-output-abc123', + outputBranch: 'sam/generated-output-abc123', + checkoutBranch: 'sam/generated-output-abc123', + baseBranch: 'main', + }, + { + scenario: 'explicit non-default branch with separate output branch', + configuredBranch: 'feature/existing-work', + outputBranch: 'sam/continuation-output-def456', + checkoutBranch: 'sam/continuation-output-def456', + baseBranch: 'feature/existing-work', + }, + { + scenario: 'explicit default branch with separate output branch', + configuredBranch: 'main', + outputBranch: 'sam/default-safe-output-ghi789', + checkoutBranch: 'sam/default-safe-output-ghi789', + baseBranch: 'main', + }, ])( - 'sends the %s in the outbound create-workspace payload', - async (_scenario, branch, outputBranch, baseBranch) => { - await handleWorkspaceDispatch(makeState(branch, outputBranch), makeContext()); + 'checks out the task output branch for $scenario', + async ({ configuredBranch, outputBranch, checkoutBranch, baseBranch }) => { + await handleWorkspaceDispatch(makeState(configuredBranch, outputBranch), makeContext()); expect(mocks.createWorkspaceOnNode).toHaveBeenCalledOnce(); expect(mocks.createWorkspaceOnNode).toHaveBeenCalledWith( 'node-1', @@ -112,7 +121,7 @@ describe('TaskRunner workspace branch dispatch', () => { 'user-1', expect.objectContaining({ workspaceId: 'workspace-1', - branch, + branch: checkoutBranch, baseBranch, defaultBranch: 'main', }) diff --git a/tasks/active/2026-07-29-safe-output-branch-dispatch.md b/tasks/active/2026-07-29-safe-output-branch-dispatch.md new file mode 100644 index 0000000000..21cb700162 --- /dev/null +++ b/tasks/active/2026-07-29-safe-output-branch-dispatch.md @@ -0,0 +1,26 @@ +# Safe output branch dispatch + +## Problem + +SAM-dispatched task work can run in a workspace checked out on the repository default branch while `tasks.output_branch` records a separate branch. VM-agent auto-commit-on-completion already has a fail-closed guard that blocks pushing when HEAD is still on the project default branch, but the safer behavior is to create task workspaces on the output branch in the first place. + +## Research findings + +- `apps/api/src/durable-objects/task-runner/workspace-steps.ts` sends the branch payload to the VM agent during workspace dispatch. Before this change it used `state.config.branch` as the checkout branch even when `state.config.outputBranch` was different. +- `packages/vm-agent/internal/server/server.go` contains an auto-commit guard that blocks default-branch pushes when HEAD equals the project default branch. Existing tests cover default-branch block and output-branch success. +- `apps/api/tests/unit/durable-objects/task-runner-workspace-branch-dispatch.test.ts` already targeted this branch payload contract, including the risky explicit-branch case. + +## Implementation checklist + +- [x] Change task-runner workspace dispatch to check out `outputBranch` when present. +- [x] Preserve explicit configured branch as the clone base when it differs from `outputBranch`. +- [x] Preserve existing non-default output branch behavior by basing generated output branches on the project default branch. +- [x] Add targeted tests covering generated output branch, explicit non-default branch with separate output branch, and explicit default branch with separate output branch. +- [x] Re-run VM-agent default-branch push guard tests to prove fail-closed protection remains intact. + +## Acceptance criteria + +- [x] Default branch is not the VM-agent checkout target when task `outputBranch` exists. +- [x] Explicit branch dispatch remains safe: explicit branch is used as the base branch, not the branch that receives task work. +- [x] Existing successful non-default branch behavior still works. +- [x] Public API contracts are unchanged; only internal task-runner-to-VM-agent payload values change. From 6ebee8c93c3d3408f96718f66623534d4161a5d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:56:51 +0000 Subject: [PATCH 02/35] fix(deploy): fail closed on workers.dev setup failure --- .github/workflows/deploy-reusable.yml | 6 +- .../quality/deploy-reusable-workflow.test.ts | 82 ++++++++++++++++++- .../2026-07-29-workersdev-cron-fail-closed.md | 34 ++++++++ 3 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 tasks/active/2026-07-29-workersdev-cron-fail-closed.md diff --git a/.github/workflows/deploy-reusable.yml b/.github/workflows/deploy-reusable.yml index 7e6ad8bc54..c667ed78e8 100644 --- a/.github/workflows/deploy-reusable.yml +++ b/.github/workflows/deploy-reusable.yml @@ -445,8 +445,10 @@ jobs: elif [ "$HTTP_CODE" -eq 409 ]; then echo "workers.dev subdomain already configured (OK)" else - echo "::warning::Failed to set workers.dev subdomain (HTTP ${HTTP_CODE}): ${BODY}" - echo "Cron triggers may not work. Initialize manually: CF Dashboard > Workers & Pages > Settings > Domains & Routes" + echo "::error::Failed to set workers.dev subdomain (HTTP ${HTTP_CODE}): ${BODY}" + echo "Deployment cannot continue because Cloudflare cron triggers require the workers.dev subdomain prerequisite." + echo "Initialize manually: CF Dashboard > Workers & Pages > Settings > Domains & Routes" + exit 1 fi # Verify Pages custom domain is active (must run BEFORE Worker deploys wildcard routes) diff --git a/scripts/quality/deploy-reusable-workflow.test.ts b/scripts/quality/deploy-reusable-workflow.test.ts index e696431d02..b75a007e7c 100644 --- a/scripts/quality/deploy-reusable-workflow.test.ts +++ b/scripts/quality/deploy-reusable-workflow.test.ts @@ -1,4 +1,7 @@ -import { readFileSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; @@ -17,6 +20,57 @@ function stepBlock(stepName: string): string { return match![0]; } +function stepRunScript(stepName: string): string { + const block = stepBlock(stepName); + const runIndex = block.indexOf(' run: |\n'); + + expect(runIndex).toBeGreaterThan(-1); + + return block + .slice(runIndex + ' run: |\n'.length) + .split('\n') + .filter((line) => line.startsWith(' ') || line.trim() === '') + .map((line) => (line.startsWith(' ') ? line.slice(' '.length) : line)) + .join('\n'); +} + +function runWorkersDevSubdomainStep(httpCode: number): { output: string; status: number } { + const tmp = mkdtempSync(join(tmpdir(), 'sam-workers-dev-test-')); + const curlPath = join(tmp, 'curl'); + + writeFileSync( + curlPath, + `#!/usr/bin/env bash\nprintf 'fake-body\\n%s\\n' "$SAM_FAKE_HTTP_CODE"\n` + ); + chmodSync(curlPath, 0o755); + + try { + const output = execFileSync('bash', ['-c', stepRunScript('Ensure workers.dev Subdomain')], { + cwd: new URL('../..', import.meta.url), + env: { + ...process.env, + PATH: `${tmp}:${process.env.PATH ?? ''}`, + CF_ACCOUNT_ID: 'account-test', + CF_API_TOKEN: 'token-test', + RESOURCE_PREFIX: 'sam-test', + SAM_FAKE_HTTP_CODE: String(httpCode), + }, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + + return { output, status: 0 }; + } catch (error) { + const execError = error as { stdout?: Buffer | string; stderr?: Buffer | string; status?: number }; + return { + output: `${execError.stdout?.toString() ?? ''}${execError.stderr?.toString() ?? ''}`, + status: execError.status ?? 1, + }; + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +} + describe('deploy reusable workflow', () => { it('runs D1 migrations and integrity checks before serving new API Worker code', () => { const backupIndex = workflow.indexOf('- name: Backup D1 Databases (pre-migration safety net)'); @@ -172,4 +226,30 @@ describe('deploy reusable workflow', () => { expect(build).toContain('make -C packages/vm-agent build-all'); expect(build).toContain('VERSION="$GITHUB_SHA"'); }); + + it('continues deployment when workers.dev subdomain setup succeeds', () => { + const result = runWorkersDevSubdomainStep(200); + + expect(result.status).toBe(0); + expect(result.output).toContain('workers.dev subdomain ready: sam-test.workers.dev'); + }); + + it('continues deployment when workers.dev subdomain is already enabled', () => { + const result = runWorkersDevSubdomainStep(409); + + expect(result.status).toBe(0); + expect(result.output).toContain('workers.dev subdomain already configured (OK)'); + }); + + it('fails closed when workers.dev subdomain setup fails', () => { + const result = runWorkersDevSubdomainStep(403); + + expect(result.status).toBe(1); + expect(result.output).toContain( + '::error::Failed to set workers.dev subdomain (HTTP 403): fake-body' + ); + expect(result.output).toContain( + 'Deployment cannot continue because Cloudflare cron triggers require the workers.dev subdomain prerequisite.' + ); + }); }); diff --git a/tasks/active/2026-07-29-workersdev-cron-fail-closed.md b/tasks/active/2026-07-29-workersdev-cron-fail-closed.md new file mode 100644 index 0000000000..04929c2a89 --- /dev/null +++ b/tasks/active/2026-07-29-workersdev-cron-fail-closed.md @@ -0,0 +1,34 @@ +# workers.dev cron setup fail-closed remediation + +## Problem + +The reusable staging/production deploy workflow currently attempts to initialize the account workers.dev subdomain before Worker deployment, but hard failures only emit a warning and allow the deploy to continue. Cloudflare cron triggers may not work unless the workers.dev subdomain prerequisite is configured, so deployment must fail closed by default or use a clearly named explicit degraded-mode override. + +## Research findings + +- `.github/workflows/deploy.yml` and `.github/workflows/deploy-staging.yml` both call `.github/workflows/deploy-reusable.yml`; the reusable workflow is the single target. +- `.github/workflows/deploy-reusable.yml` step `Ensure workers.dev Subdomain` treats 2xx and 409 as success, but warns/continues for every other HTTP result. +- Existing workflow static tests live in `scripts/quality/deploy-reusable-workflow.test.ts`. +- Related deployment hardening task `tasks/archive/2026-07-18-safe-d1-migration-deploy-order.md` uses the same static workflow-test pattern for deployment gates. +- Public docs mention self-hosting/deployment configuration, but this fix preserves default success behavior and only changes failed prerequisite handling; public docs are only needed if an explicit operator override is added. + +## Implementation checklist + +- [x] Change `Ensure workers.dev Subdomain` to fail closed by default for non-2xx/non-409 responses. +- [x] Add a clearly named degraded-mode override only if needed. +- [x] Preserve successful 2xx deploy behavior. +- [x] Preserve 409 already-configured deploy behavior. +- [x] Add tests for success, 409 already-enabled, hard failure, and explicit override if implemented. +- [x] Run targeted workflow quality tests. +- [x] Run broader relevant validation. +- [x] Complete local Cloudflare/security/test/task-completion reviews. +- [ ] Open PR and do not merge. + +## Acceptance criteria + +- Deployment exits non-zero by default when workers.dev subdomain setup cannot be configured/verified. +- 2xx and 409 responses continue to pass. +- Any degraded-mode path is explicitly named and opt-in. +- Static tests prove the shell behavior branches. +- PR includes test evidence, CI status, and no-breaking-change/deploy-risk rationale. + From b18ec914bcfd0299190d522537bc189e2bb47618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:37:45 +0000 Subject: [PATCH 03/35] task: archive safe output branch dispatch --- .../{active => archive}/2026-07-29-safe-output-branch-dispatch.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tasks/{active => archive}/2026-07-29-safe-output-branch-dispatch.md (100%) diff --git a/tasks/active/2026-07-29-safe-output-branch-dispatch.md b/tasks/archive/2026-07-29-safe-output-branch-dispatch.md similarity index 100% rename from tasks/active/2026-07-29-safe-output-branch-dispatch.md rename to tasks/archive/2026-07-29-safe-output-branch-dispatch.md From 21059fe8cf23295796565bf984f8aaf543c2f071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:55:56 +0000 Subject: [PATCH 04/35] fix: keep wrangler sync env parity --- .claude/rules/07-env-and-urls.md | 4 + .github/workflows/deploy-reusable.yml | 18 ++--- .../quality/deploy-reusable-workflow.test.ts | 77 +++++++++++++++---- .../2026-07-29-wrangler-sync-env-parity.md | 55 +++++++++++++ 4 files changed, 129 insertions(+), 25 deletions(-) create mode 100644 tasks/archive/2026-07-29-wrangler-sync-env-parity.md diff --git a/.claude/rules/07-env-and-urls.md b/.claude/rules/07-env-and-urls.md index 37bba3aa9e..c3269b1105 100644 --- a/.claude/rules/07-env-and-urls.md +++ b/.claude/rules/07-env-and-urls.md @@ -74,6 +74,10 @@ The CI quality check (`pnpm quality:wrangler-bindings`) verifies: 1. No `[env.*]` sections exist in checked-in `wrangler.toml` files 2. All required binding types are present at the top level +### Required action when adding a Worker var consumed by sync-wrangler-config + +If `scripts/deploy/sync-wrangler-config.ts` reads a GitHub Environment variable from `process.env` to generate Worker `[vars]`, add it to the centralized `wrangler_sync_env` mapping in `.github/workflows/deploy-reusable.yml`. Do not add ad hoc per-step sync env blocks. First deploys run the sync script twice (initial sync, then tail-consumer re-sync), and both invocations must receive identical optional Worker env inputs so the second sync cannot silently drop operator overrides or restore script defaults. + ### Why this architecture Wrangler does NOT inherit bindings (D1, KV, R2, DO, AI, tail_consumers) from top-level into `[env.*]` sections. Previously, this required manually duplicating every binding 3x (top-level + staging + production). Now the sync script generates complete env sections, eliminating duplication and making the config fork-friendly. diff --git a/.github/workflows/deploy-reusable.yml b/.github/workflows/deploy-reusable.yml index 7e6ad8bc54..f23d72759c 100644 --- a/.github/workflows/deploy-reusable.yml +++ b/.github/workflows/deploy-reusable.yml @@ -347,7 +347,7 @@ jobs: - name: Sync Wrangler Config (API + Tail Worker) if: ${{ inputs.dry_run != true }} run: pnpm tsx scripts/deploy/sync-wrangler-config.ts - env: + env: &wrangler_sync_env PULUMI_STACK: ${{ steps.pulumi-select.outputs.stack_name }} AWS_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.R2_SECRET_ACCESS_KEY }} @@ -357,6 +357,7 @@ jobs: BASE_DOMAIN: ${{ vars.BASE_DOMAIN }} RESOURCE_PREFIX: ${{ steps.prefix.outputs.value }} REQUIRE_APPROVAL: ${{ vars.REQUIRE_APPROVAL }} + SETUP_FORCE: ${{ vars.SETUP_FORCE }} HETZNER_BASE_IMAGE: ${{ vars.HETZNER_BASE_IMAGE }} PLATFORM_FEEDBACK_PROJECT_ID: ${{ vars.PLATFORM_FEEDBACK_PROJECT_ID }} REPORT_ISSUE_TITLE_MAX_LENGTH: ${{ vars.REPORT_ISSUE_TITLE_MAX_LENGTH }} @@ -371,8 +372,11 @@ jobs: ARTIFACTS_BINDING_ENABLED: ${{ vars.ARTIFACTS_BINDING_ENABLED }} CF_CONTAINER_ENABLED: ${{ vars.CF_CONTAINER_ENABLED }} CF_CONTAINER_SLEEP_AFTER: ${{ vars.CF_CONTAINER_SLEEP_AFTER }} + CF_CONTAINER_ACTIVE_WORK_MAX_MS: ${{ vars.CF_CONTAINER_ACTIVE_WORK_MAX_MS }} + CF_CONTAINER_KEEPALIVE_RENEW_INTERVAL_MS: ${{ vars.CF_CONTAINER_KEEPALIVE_RENEW_INTERVAL_MS }} CF_CONTAINER_PORT_READY_TIMEOUT_MS: ${{ vars.CF_CONTAINER_PORT_READY_TIMEOUT_MS }} CF_CONTAINER_WAKE_TIMEOUT_MS: ${{ vars.CF_CONTAINER_WAKE_TIMEOUT_MS }} + CF_CONTAINER_RECOVERY_MAX_ATTEMPTS: ${{ vars.CF_CONTAINER_RECOVERY_MAX_ATTEMPTS }} CF_CONTAINER_CREATE_WORKSPACE_TIMEOUT_MS: ${{ vars.CF_CONTAINER_CREATE_WORKSPACE_TIMEOUT_MS }} CF_CONTAINER_CLONE_FILTER: ${{ vars.CF_CONTAINER_CLONE_FILTER }} CF_CONTAINER_VM_AGENT_PORT: ${{ vars.CF_CONTAINER_VM_AGENT_PORT }} @@ -675,17 +679,7 @@ jobs: rm -f .wrangler/tail-worker-first-deploy pnpm tsx scripts/deploy/sync-wrangler-config.ts env: - PULUMI_STACK: ${{ steps.pulumi-select.outputs.stack_name }} - AWS_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }} - AWS_SECRET_ACCESS_KEY: ${{ secrets.R2_SECRET_ACCESS_KEY }} - PULUMI_CONFIG_PASSPHRASE: ${{ secrets.PULUMI_CONFIG_PASSPHRASE }} - CF_API_TOKEN: ${{ secrets.CF_API_TOKEN }} - CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }} - BASE_DOMAIN: ${{ vars.BASE_DOMAIN }} - RESOURCE_PREFIX: ${{ steps.prefix.outputs.value }} - REQUIRE_APPROVAL: ${{ vars.REQUIRE_APPROVAL }} - HETZNER_BASE_IMAGE: ${{ vars.HETZNER_BASE_IMAGE }} - ARTIFACTS_BINDING_ENABLED: ${{ vars.ARTIFACTS_BINDING_ENABLED }} + <<: *wrangler_sync_env - name: Re-deploy API Worker (with tail_consumers) if: ${{ inputs.dry_run != true && steps.first_deploy.outputs.is_first == 'true' }} diff --git a/scripts/quality/deploy-reusable-workflow.test.ts b/scripts/quality/deploy-reusable-workflow.test.ts index e696431d02..d99c6caf87 100644 --- a/scripts/quality/deploy-reusable-workflow.test.ts +++ b/scripts/quality/deploy-reusable-workflow.test.ts @@ -6,17 +6,51 @@ const workflow = readFileSync( new URL('../../.github/workflows/deploy-reusable.yml', import.meta.url), 'utf8' ); +const syncWranglerConfig = readFileSync( + new URL('../deploy/sync-wrangler-config.ts', import.meta.url), + 'utf8' +); function stepBlock(stepName: string): string { const pattern = new RegExp( String.raw` - name: ${stepName}[\s\S]*?(?=\n - name:|\n #|$)` ); - const match = workflow.match(pattern); + const block = workflow.match(pattern)?.[0]; + + expect(block).toBeDefined(); + if (!block) { + throw new Error(`Unable to find workflow step: ${stepName}`); + } + return block; +} + +function extractOptionalWorkerEnvVars(): string[] { + const match = syncWranglerConfig.match(/getOptionalProcessEnvVars\(\[\s*([\s\S]*?)\s*\]\)/); + const optionalEnvBlock = match?.[1]; - expect(match?.[0]).toBeDefined(); - return match![0]; + expect(optionalEnvBlock).toBeDefined(); + if (!optionalEnvBlock) { + throw new Error('Unable to find sync-wrangler optional Worker env var list'); + } + + const vars = Array.from(optionalEnvBlock.matchAll(/'([A-Z0-9_]+)'/g), (varMatch) => varMatch[1]); + expect(vars).toContain('CF_CONTAINER_ENABLED'); + expect(vars).toContain('SANDBOX_ENABLED'); + expect(vars).toContain('MAX_CONCURRENT_SETUP_SESSIONS'); + + return vars; } +const DIRECT_SYNC_ENV_MAPPINGS = { + PULUMI_STACK: 'PULUMI_STACK: ${{ steps.pulumi-select.outputs.stack_name }}', + CF_API_TOKEN: 'CF_API_TOKEN: ${{ secrets.CF_API_TOKEN }}', + CLOUDFLARE_API_TOKEN: 'CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }}', + ARTIFACTS_BINDING_ENABLED: 'ARTIFACTS_BINDING_ENABLED: ${{ vars.ARTIFACTS_BINDING_ENABLED }}', + SETUP_FORCE: 'SETUP_FORCE: ${{ vars.SETUP_FORCE }}', + BASE_DOMAIN: 'BASE_DOMAIN: ${{ vars.BASE_DOMAIN }}', + RESOURCE_PREFIX: 'RESOURCE_PREFIX: ${{ steps.prefix.outputs.value }}', +} as const; + describe('deploy reusable workflow', () => { it('runs D1 migrations and integrity checks before serving new API Worker code', () => { const backupIndex = workflow.indexOf('- name: Backup D1 Databases (pre-migration safety net)'); @@ -40,17 +74,34 @@ describe('deploy reusable workflow', () => { expect(redeployAfterSecretsIndex).toBeGreaterThan(deployApiIndex); }); - it('passes derived deployment identity into every Wrangler config sync phase', () => { - for (const name of [ - 'Sync Wrangler Config \\(API \\+ Tail Worker\\)', - 'Re-sync Wrangler Config \\(add tail_consumers\\)', - ]) { - const block = stepBlock(name); + it('passes derived deployment identity through the shared Wrangler config sync env', () => { + const initialSync = stepBlock('Sync Wrangler Config \\(API \\+ Tail Worker\\)'); + const firstDeployResync = stepBlock('Re-sync Wrangler Config \\(add tail_consumers\\)'); - expect(block).toContain('pnpm tsx scripts/deploy/sync-wrangler-config.ts'); - expect(block).toContain('BASE_DOMAIN: ${{ vars.BASE_DOMAIN }}'); - expect(block).toContain('RESOURCE_PREFIX: ${{ steps.prefix.outputs.value }}'); - expect(block).toContain('ARTIFACTS_BINDING_ENABLED: ${{ vars.ARTIFACTS_BINDING_ENABLED }}'); + expect(initialSync).toContain('pnpm tsx scripts/deploy/sync-wrangler-config.ts'); + expect(initialSync).toContain('BASE_DOMAIN: ${{ vars.BASE_DOMAIN }}'); + expect(initialSync).toContain('RESOURCE_PREFIX: ${{ steps.prefix.outputs.value }}'); + expect(initialSync).toContain( + 'ARTIFACTS_BINDING_ENABLED: ${{ vars.ARTIFACTS_BINDING_ENABLED }}' + ); + + expect(firstDeployResync).toContain('pnpm tsx scripts/deploy/sync-wrangler-config.ts'); + expect(firstDeployResync).toContain('<<: *wrangler_sync_env'); + }); + + it('uses one complete env mapping for every Wrangler config sync invocation', () => { + const initialSync = stepBlock('Sync Wrangler Config \\(API \\+ Tail Worker\\)'); + const firstDeployResync = stepBlock('Re-sync Wrangler Config \\(add tail_consumers\\)'); + + expect(initialSync).toContain('env: &wrangler_sync_env'); + expect(firstDeployResync).toContain('<<: *wrangler_sync_env'); + + for (const mapping of Object.values(DIRECT_SYNC_ENV_MAPPINGS)) { + expect(initialSync).toContain(mapping); + } + + for (const envVar of extractOptionalWorkerEnvVars()) { + expect(initialSync).toContain(`${envVar}: \${{ vars.${envVar} }}`); } }); diff --git a/tasks/archive/2026-07-29-wrangler-sync-env-parity.md b/tasks/archive/2026-07-29-wrangler-sync-env-parity.md new file mode 100644 index 0000000000..cfbeb95f2f --- /dev/null +++ b/tasks/archive/2026-07-29-wrangler-sync-env-parity.md @@ -0,0 +1,55 @@ +# Wrangler sync env parity for first deploy + +## Problem statement + +`.github/workflows/deploy-reusable.yml` runs `scripts/deploy/sync-wrangler-config.ts` twice on first deploys: the initial config sync and a re-sync after the tail worker exists. The initial sync passes cf-container, sandbox, and setup tuning environment variables, but the first-deploy re-sync omits them. Because `sync-wrangler-config.ts` reads those values from `process.env` and defaults `CF_CONTAINER_ENABLED` to `true`, a first deploy can silently re-enable Cloudflare Containers or drop operator-provided timeout/capacity tunables. + +This PR must be tightly scoped: make the mapping deterministic and test it. + +## Research findings + +- `scripts/deploy/sync-wrangler-config.ts` builds Worker vars in `getApiWorkerVars()`. +- Optional Worker vars consumed by the script are listed in the `getOptionalProcessEnvVars([...])` call. +- The workflow has two `pnpm tsx scripts/deploy/sync-wrangler-config.ts` invocations: + - `Sync Wrangler Config (API + Tail Worker)` + - `Re-sync Wrangler Config (add tail_consumers)` +- Existing quality tests in `scripts/quality/deploy-reusable-workflow.test.ts` already inspect workflow step blocks and are the right place for a deterministic regression test. +- `.claude/rules/07-env-and-urls.md` records that wrangler env sections are generated at deploy time and that Miniflare tests do not catch `wrangler.toml` generation mistakes. +- Prior incident lesson: `tasks/archive/2026-07-19-fix-instant-container-clone-timeout.md` notes env-validator findings where new cf-container env vars were not wired through deploy pipeline allowlists. + +## Checklist + +- [x] Centralize the sync env mapping for all `sync-wrangler-config.ts` workflow invocations. +- [x] Ensure first sync and first-deploy re-sync receive identical optional Worker env inputs. +- [x] Include all env vars consumed by `getOptionalProcessEnvVars()`, including currently missing `CF_CONTAINER_ACTIVE_WORK_MAX_MS`, `CF_CONTAINER_KEEPALIVE_RENEW_INTERVAL_MS`, and `CF_CONTAINER_RECOVERY_MAX_ATTEMPTS`. +- [x] Preserve existing behavior when GitHub vars/secrets are absent. +- [x] Add a workflow quality test that derives consumed optional env vars from `sync-wrangler-config.ts` and fails if any sync invocation does not use the centralized mapping. +- [x] Add a process note to prevent future duplicated deploy sync env blocks. +- [x] Run relevant local tests and full quality gates. +- [x] Run local env-validator and test-engineer reviews before PR completion. +- [ ] Open a narrow PR and do not merge. + +## Acceptance criteria + +- Both deploy-reusable sync invocations use the same env mapping for `sync-wrangler-config.ts`. +- A test fails if a sync invocation omits any direct sync env or optional Worker var consumed by `sync-wrangler-config.ts`. +- Defaults and behavior remain unchanged when vars are absent. +- CI is green on the PR. +- PR is left open/unmerged. + +## Post-mortem + +- **What broke**: First deploys could re-run wrangler config generation without the same operator overrides used by the initial sync. That could produce a different API Worker env section during the tail-consumer re-sync. +- **Root cause**: The workflow duplicated the env mapping inline for each sync invocation, and the second block drifted behind the first. +- **Timeline**: The drift was found by strict CTO infra review task `01KYQHJJM4W83JKKDKTTPA9CNF` and remediated in this task. +- **Why it wasn't caught**: Existing workflow quality tests checked selected vars on the initial sync only and did not derive expected coverage from the env vars consumed by `sync-wrangler-config.ts`. +- **Class of bug**: Duplicated deployment env mappings across multi-phase deploy workflows. +- **Process fix**: Add a project rule note and a regression test that requires sync invocations to share a centralized mapping and keeps it aligned with `sync-wrangler-config.ts`. + +## References + +- `.github/workflows/deploy-reusable.yml` +- `scripts/deploy/sync-wrangler-config.ts` +- `scripts/quality/deploy-reusable-workflow.test.ts` +- `.claude/rules/07-env-and-urls.md` +- `tasks/archive/2026-07-19-fix-instant-container-clone-timeout.md` From 4517b3e9c4308658d8d5eb3c7217455dae733bc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:05:36 +0000 Subject: [PATCH 05/35] chore: refresh CI after PR body correction From 35d3510e11dd29f60a88e84b20c305ea5955aed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:30:11 +0000 Subject: [PATCH 06/35] task: add atomic bootstrap token redemption --- ...07-29-atomic-bootstrap-token-redemption.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tasks/backlog/2026-07-29-atomic-bootstrap-token-redemption.md diff --git a/tasks/backlog/2026-07-29-atomic-bootstrap-token-redemption.md b/tasks/backlog/2026-07-29-atomic-bootstrap-token-redemption.md new file mode 100644 index 0000000000..26fc124c0e --- /dev/null +++ b/tasks/backlog/2026-07-29-atomic-bootstrap-token-redemption.md @@ -0,0 +1,37 @@ +# Atomic bootstrap token redemption + +## Problem + +Bootstrap token redemption currently depends on KV `get` then `delete`, plus an isolate-local in-flight map. That prevents duplicate redemption inside one isolate but does not make the single-use consume atomic across Cloudflare Worker isolates or concurrent requests. + +## Research findings + +- `apps/api/src/services/bootstrap.ts` stores token payloads in KV and redeems with KV `get/delete`. +- `apps/api/src/routes/bootstrap.ts` exposes `POST /api/bootstrap/:token` and preserves the current response shape for VM agents. +- `apps/api/src/routes/workspaces/runtime.ts` has a legacy bootstrap-token endpoint that writes KV directly. +- Existing tests cover normal valid/invalid redemption and an isolate-local in-flight replay, but not cross-isolate atomic consume. +- Cloudflare KV cannot provide compare-and-delete semantics; D1 can provide an atomic conditional write/update for the consume decision. +- Relevant prior findings: + - `tasks/archive/2026-03-12-shannon-security-assessment.md` documents the TOCTOU race on bootstrap KV get/delete. + - `tasks/archive/2026-07-18-harden-callback-bootstrap-token-lifecycle.md` requires fail-closed callback/bootstrap token lifecycle behavior while keeping bounded legacy compatibility. + +## Checklist + +- [ ] Add a D1 migration for a bootstrap token consume ledger keyed by a non-secret token hash. +- [ ] Register newly created bootstrap tokens in the ledger while preserving the existing KV payload format. +- [ ] Redeem tokens only after an atomic D1 consume succeeds. +- [ ] Add migration-safe handling for in-flight legacy KV-only tokens, using atomic insert-wins semantics. +- [ ] Fail closed if the consume state is ambiguous or failed. +- [ ] Update direct legacy bootstrap-token creation to register the ledger. +- [ ] Add tests for concurrent redemption, single-use semantics, expired/missing tokens, and existing valid-token compatibility. +- [ ] Run relevant validation and local specialist reviews. +- [ ] Open a narrow PR and do not merge. + +## Acceptance criteria + +- Exactly one concurrent redemption can receive credentials for a known valid token. +- Replays are rejected after one successful consume. +- Missing and expired tokens are rejected without exposing credentials. +- Existing valid token response semantics and token format remain unchanged. +- In-flight KV-only tokens remain redeemable at most once through an atomic legacy claim path. +- CI is green and the PR documents no-breaking-change rationale. From a6cbc03db80d4cdf1eaee4fcb2a354702e338ff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:30:29 +0000 Subject: [PATCH 07/35] task: activate atomic bootstrap token redemption --- .../2026-07-29-atomic-bootstrap-token-redemption.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tasks/{backlog => active}/2026-07-29-atomic-bootstrap-token-redemption.md (100%) diff --git a/tasks/backlog/2026-07-29-atomic-bootstrap-token-redemption.md b/tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md similarity index 100% rename from tasks/backlog/2026-07-29-atomic-bootstrap-token-redemption.md rename to tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md From 1270432fd5df098ed26698ef8cf1081dc7bec5f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:37:52 +0000 Subject: [PATCH 08/35] fix: make bootstrap token redemption atomic --- .../0101_bootstrap_token_consumes.sql | 8 + apps/api/src/routes/workspaces/runtime.ts | 7 + apps/api/src/services/bootstrap.ts | 170 +++++++++++++----- apps/api/tests/unit/routes/bootstrap.test.ts | 37 +++- .../api/tests/unit/services/bootstrap.test.ts | 163 ++++++++++++++++- ...07-29-atomic-bootstrap-token-redemption.md | 14 +- 6 files changed, 337 insertions(+), 62 deletions(-) create mode 100644 apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql diff --git a/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql b/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql new file mode 100644 index 0000000000..f0433906f4 --- /dev/null +++ b/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql @@ -0,0 +1,8 @@ +CREATE TABLE IF NOT EXISTS bootstrap_token_consumes ( + token_hash TEXT PRIMARY KEY NOT NULL, + created_at TEXT NOT NULL, + expires_at TEXT NOT NULL, + consumed_at TEXT +); +CREATE INDEX IF NOT EXISTS idx_bootstrap_token_consumes_expiry ON bootstrap_token_consumes(expires_at); +CREATE INDEX IF NOT EXISTS idx_bootstrap_token_consumes_consumed ON bootstrap_token_consumes(consumed_at); diff --git a/apps/api/src/routes/workspaces/runtime.ts b/apps/api/src/routes/workspaces/runtime.ts index 020ece6418..fa73d6bb89 100644 --- a/apps/api/src/routes/workspaces/runtime.ts +++ b/apps/api/src/routes/workspaces/runtime.ts @@ -33,6 +33,7 @@ import { MessageBatchSchema, } from '../../schemas'; import { appendBootLog } from '../../services/boot-log'; +import { registerBootstrapTokenConsume } from '../../services/bootstrap'; import { syncActiveAgentCredentialSecret } from '../../services/composable-credentials/agent-sync'; import { decrypt, encrypt } from '../../services/encryption'; import { getInstallationToken, getUserInstallationRepositories } from '../../services/github-app'; @@ -1568,6 +1569,12 @@ runtimeRoutes.post('/:id/bootstrap-token', requireAuth(), requireApproved(), asy createdAt: now, }; + await registerBootstrapTokenConsume( + c.env.DATABASE, + bootstrapToken, + new Date(Date.now() + 60 * 1000).toISOString() + ); + await c.env.KV.put(`bootstrap:${bootstrapToken}`, JSON.stringify(data), { expirationTtl: 60, }); diff --git a/apps/api/src/services/bootstrap.ts b/apps/api/src/services/bootstrap.ts index 835173b101..78f6b5f1f7 100644 --- a/apps/api/src/services/bootstrap.ts +++ b/apps/api/src/services/bootstrap.ts @@ -13,7 +13,6 @@ import { decrypt, encrypt } from './encryption'; /** KV key prefix for bootstrap tokens */ const BOOTSTRAP_PREFIX = 'bootstrap:'; -const inFlightRedemptions = new Map>(); /** Default bootstrap token TTL in seconds (15 minutes) */ const DEFAULT_BOOTSTRAP_TTL = 900; @@ -23,6 +22,7 @@ interface BootstrapEnv { BOOTSTRAP_TOKEN_TTL_SECONDS?: string; ENCRYPTION_KEY: string; CREDENTIAL_ENCRYPTION_KEY?: string; + DATABASE: D1Database; } /** Get bootstrap TTL from env or use default (per constitution principle XI) */ @@ -74,6 +74,8 @@ export async function storeBootstrapToken( callbackTokenIv: encryptedCallbackToken.iv, }; + await registerBootstrapTokenConsume(env.DATABASE, token, nowPlusSeconds(ttl)); + await kv.put(`${BOOTSTRAP_PREFIX}${token}`, JSON.stringify(storedData), { expirationTtl: ttl, }); @@ -94,61 +96,143 @@ export async function redeemBootstrapToken( env: BootstrapEnv ): Promise { const key = `${BOOTSTRAP_PREFIX}${token}`; - const existing = inFlightRedemptions.get(key); - if (existing) { + const consumeState = await reserveBootstrapTokenConsume(env.DATABASE, token); + if (consumeState === 'rejected') { return null; } - const redemption = (async () => { - const data = await kv.get(key, { type: 'json' }); + const data = await kv.get(key, { type: 'json' }); + if (!data) { + return null; + } - if (!data) { + if (consumeState === 'legacy-claim-required') { + const claimed = await claimLegacyBootstrapTokenConsume(env.DATABASE, token, env); + if (!claimed) { return null; } + } - // Delete immediately to enforce single-use before decrypting or returning credentials. - await kv.delete(key); - - if (data.encryptedCallbackToken && data.callbackTokenIv) { - const callbackToken = await decrypt( - data.encryptedCallbackToken, - data.callbackTokenIv, - getCredentialEncryptionKey(env) - ); + // Delete after the D1 consume decision. If payload handling later fails, the D1 + // row remains consumed so ambiguous/failed redemption is fail-closed. + await kv.delete(key); + + if (data.encryptedCallbackToken && data.callbackTokenIv) { + const callbackToken = await decrypt( + data.encryptedCallbackToken, + data.callbackTokenIv, + getCredentialEncryptionKey(env) + ); + + return { + ...data, + callbackToken, + }; + } - return { - ...data, - callbackToken, - }; - } + // Backward compatibility for bootstrap entries written before callback token encryption. + // This is intentionally bounded to entries still inside the configured bootstrap TTL + // plus a small clock-skew allowance; older plaintext records fail closed. + if (data.callbackToken && isLegacyPlaintextCallbackTokenStillRedeemable(data, env)) { + log.warn('bootstrap.legacy_plaintext_callback_token_redeemed', { + workspaceId: data.workspaceId, + createdAt: data.createdAt, + }); + return data; + } - // Backward compatibility for bootstrap entries written before callback token encryption. - // This is intentionally bounded to entries still inside the configured bootstrap TTL - // plus a small clock-skew allowance; older plaintext records fail closed. - if (data.callbackToken && isLegacyPlaintextCallbackTokenStillRedeemable(data, env)) { - log.warn('bootstrap.legacy_plaintext_callback_token_redeemed', { - workspaceId: data.workspaceId, - createdAt: data.createdAt, - }); - return data; - } + if (data.callbackToken) { + log.warn('bootstrap.legacy_plaintext_callback_token_rejected', { + workspaceId: data.workspaceId, + createdAt: data.createdAt, + }); + } - if (data.callbackToken) { - log.warn('bootstrap.legacy_plaintext_callback_token_rejected', { - workspaceId: data.workspaceId, - createdAt: data.createdAt, - }); - } + throw new Error('Bootstrap token data is missing callback token material'); +} - throw new Error('Bootstrap token data is missing callback token material'); - })(); +export async function registerBootstrapTokenConsume( + db: D1Database, + token: string, + expiresAt: string +): Promise { + await db + .prepare( + `INSERT INTO bootstrap_token_consumes (token_hash, created_at, expires_at) + VALUES (?, ?, ?)` + ) + .bind(await hashBootstrapToken(token), new Date().toISOString(), expiresAt) + .run(); +} - inFlightRedemptions.set(key, redemption); - try { - return await redemption; - } finally { - inFlightRedemptions.delete(key); +type BootstrapConsumeState = 'consumed' | 'legacy-claim-required' | 'rejected'; + +async function reserveBootstrapTokenConsume( + db: D1Database, + token: string +): Promise { + const now = new Date().toISOString(); + const tokenHash = await hashBootstrapToken(token); + + const updated = await db + .prepare( + `UPDATE bootstrap_token_consumes + SET consumed_at = ? + WHERE token_hash = ? + AND consumed_at IS NULL + AND expires_at > ?` + ) + .bind(now, tokenHash, now) + .run(); + + if (d1Changes(updated) === 1) { + return 'consumed'; } + + const existing = await db + .prepare('SELECT token_hash FROM bootstrap_token_consumes WHERE token_hash = ?') + .bind(tokenHash) + .first('token_hash'); + + return existing ? 'rejected' : 'legacy-claim-required'; +} + +async function claimLegacyBootstrapTokenConsume( + db: D1Database, + token: string, + env: Pick +): Promise { + const now = new Date().toISOString(); + + // Migration-safe compatibility for unexpired tokens that were written to KV + // before the D1 ledger existed, or direct legacy producers that only wrote KV. + // The unique token_hash insert is the atomic cross-isolate claim: one request + // can create the consumed row, all duplicates fail closed. + const legacyClaim = await db + .prepare( + `INSERT OR IGNORE INTO bootstrap_token_consumes (token_hash, created_at, expires_at, consumed_at) + VALUES (?, ?, ?, ?)` + ) + .bind(await hashBootstrapToken(token), now, nowPlusSeconds(getBootstrapTTL(env)), now) + .run(); + + return d1Changes(legacyClaim) === 1; +} + +function d1Changes(result: D1Result): number { + return typeof result.meta?.changes === 'number' ? result.meta.changes : 0; +} + +async function hashBootstrapToken(token: string): Promise { + const bytes = new TextEncoder().encode(token); + const digest = await crypto.subtle.digest('SHA-256', bytes); + return [...new Uint8Array(digest)] + .map((byte) => byte.toString(16).padStart(2, '0')) + .join(''); +} + +function nowPlusSeconds(seconds: number): string { + return new Date(Date.now() + seconds * 1000).toISOString(); } function isLegacyPlaintextCallbackTokenStillRedeemable( diff --git a/apps/api/tests/unit/routes/bootstrap.test.ts b/apps/api/tests/unit/routes/bootstrap.test.ts index ebe03a776d..e8799235fd 100644 --- a/apps/api/tests/unit/routes/bootstrap.test.ts +++ b/apps/api/tests/unit/routes/bootstrap.test.ts @@ -1,6 +1,9 @@ import type { BootstrapResponse, BootstrapTokenData } from '@simple-agent-manager/shared'; +import Database from 'better-sqlite3'; import { Hono } from 'hono'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createSqliteD1 } from '../../helpers/sqlite-d1'; // Mock rate-limit middleware to be a passthrough (tested separately) vi.mock('../../../src/middleware/rate-limit', () => ({ @@ -18,14 +21,34 @@ const mockKV = { // Mock environment const mockEnv = { KV: mockKV, - DATABASE: {}, + DATABASE: undefined as unknown as D1Database, ENCRYPTION_KEY: 'iZEI8rg5FHtTo2yvt6Qw3m4z6aTfqj5MdLEGqOvdqw0=', // Valid 32-byte base64 key BASE_DOMAIN: 'workspaces.example.com', }; +let sqlite: Database.Database; + +function installBootstrapLedger(db: Database.Database): void { + db.exec(` + CREATE TABLE bootstrap_token_consumes ( + token_hash TEXT PRIMARY KEY NOT NULL, + created_at TEXT NOT NULL, + expires_at TEXT NOT NULL, + consumed_at TEXT + ); + `); +} + describe('Bootstrap Routes', () => { beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); + sqlite = new Database(':memory:'); + installBootstrapLedger(sqlite); + mockEnv.DATABASE = createSqliteD1(sqlite); + }); + + afterEach(() => { + sqlite.close(); }); describe('POST /api/bootstrap/:token', () => { @@ -193,7 +216,7 @@ describe('Bootstrap Routes', () => { expect(body.gitUserEmail).toBeNull(); }); - it('rejects concurrent replay while first redemption is in flight', async () => { + it('rejects concurrent replay across requests using the D1 consume ledger', async () => { const { bootstrapRoutes } = await import('../../../src/routes/bootstrap'); const { encrypt } = await import('../../../src/services/encryption'); @@ -213,14 +236,10 @@ describe('Bootstrap Routes', () => { createdAt: new Date().toISOString(), }; - let releaseGet!: () => void; - mockKV.get.mockReturnValueOnce(new Promise((resolve) => { - releaseGet = () => resolve(tokenData); - })); + mockKV.get.mockResolvedValue(tokenData); const first = app.request('/api/bootstrap/concurrent-token', { method: 'POST' }, mockEnv); const second = app.request('/api/bootstrap/concurrent-token', { method: 'POST' }, mockEnv); - releaseGet(); const [res1, res2] = await Promise.all([first, second]); expect([res1.status, res2.status].sort()).toEqual([200, 401]); diff --git a/apps/api/tests/unit/services/bootstrap.test.ts b/apps/api/tests/unit/services/bootstrap.test.ts index 6e8a014bff..2695384b84 100644 --- a/apps/api/tests/unit/services/bootstrap.test.ts +++ b/apps/api/tests/unit/services/bootstrap.test.ts @@ -1,5 +1,8 @@ import type { BootstrapTokenData } from '@simple-agent-manager/shared'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import Database from 'better-sqlite3'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createSqliteD1 } from '../../helpers/sqlite-d1'; // Mock KV namespace const mockKV = { @@ -10,11 +13,37 @@ const mockKV = { const mockEnv = { ENCRYPTION_KEY: 'iZEI8rg5FHtTo2yvt6Qw3m4z6aTfqj5MdLEGqOvdqw0=', + DATABASE: undefined as unknown as D1Database, }; +let sqlite: Database.Database; + +function installBootstrapLedger(db: Database.Database): void { + db.exec(` + CREATE TABLE bootstrap_token_consumes ( + token_hash TEXT PRIMARY KEY NOT NULL, + created_at TEXT NOT NULL, + expires_at TEXT NOT NULL, + consumed_at TEXT + ); + `); +} + +async function tokenHash(token: string): Promise { + const digest = await crypto.subtle.digest('SHA-256', new TextEncoder().encode(token)); + return [...new Uint8Array(digest)].map((byte) => byte.toString(16).padStart(2, '0')).join(''); +} + describe('Bootstrap Service', () => { beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); + sqlite = new Database(':memory:'); + installBootstrapLedger(sqlite); + mockEnv.DATABASE = createSqliteD1(sqlite); + }); + + afterEach(() => { + sqlite.close(); }); describe('generateBootstrapToken', () => { @@ -79,7 +108,7 @@ describe('Bootstrap Service', () => { }); }); - describe('redeemBootstrapToken (get + delete for single-use)', () => { + describe('redeemBootstrapToken (D1 atomic consume + KV payload)', () => { it('should return null for non-existent token', async () => { const { redeemBootstrapToken } = await import( '../../../src/services/bootstrap' @@ -139,6 +168,134 @@ describe('Bootstrap Service', () => { // Token should be deleted after redemption (single-use) expect(mockKV.delete).toHaveBeenCalledWith('bootstrap:valid-token'); }); + + it('allows exactly one concurrent redemption across requests', async () => { + const { redeemBootstrapToken, registerBootstrapTokenConsume } = await import( + '../../../src/services/bootstrap' + ); + const { encrypt } = await import('../../../src/services/encryption'); + + const encryptedCallbackToken = await encrypt('jwt-callback-token', mockEnv.ENCRYPTION_KEY); + const data: BootstrapTokenData = { + workspaceId: 'ws-atomic', + encryptedHetznerToken: 'encrypted-hetzner', + hetznerTokenIv: 'hetzner-iv', + encryptedCallbackToken: encryptedCallbackToken.ciphertext, + callbackTokenIv: encryptedCallbackToken.iv, + encryptedGithubToken: null, + githubTokenIv: null, + createdAt: new Date().toISOString(), + }; + + await registerBootstrapTokenConsume( + mockEnv.DATABASE, + 'atomic-token', + new Date(Date.now() + 60_000).toISOString() + ); + mockKV.get.mockResolvedValue(data); + + const results = await Promise.all([ + redeemBootstrapToken(mockKV as unknown as KVNamespace, 'atomic-token', mockEnv), + redeemBootstrapToken(mockKV as unknown as KVNamespace, 'atomic-token', mockEnv), + ]); + + expect(results.filter(Boolean)).toHaveLength(1); + expect(results.filter((result) => result === null)).toHaveLength(1); + expect(mockKV.delete).toHaveBeenCalledTimes(1); + }); + + it('rejects replay after a successful registered-token consume', async () => { + const { redeemBootstrapToken, registerBootstrapTokenConsume } = await import( + '../../../src/services/bootstrap' + ); + const { encrypt } = await import('../../../src/services/encryption'); + + const encryptedCallbackToken = await encrypt('jwt-callback-token', mockEnv.ENCRYPTION_KEY); + const data: BootstrapTokenData = { + workspaceId: 'ws-once', + encryptedHetznerToken: 'encrypted-hetzner', + hetznerTokenIv: 'hetzner-iv', + encryptedCallbackToken: encryptedCallbackToken.ciphertext, + callbackTokenIv: encryptedCallbackToken.iv, + encryptedGithubToken: null, + githubTokenIv: null, + createdAt: new Date().toISOString(), + }; + + await registerBootstrapTokenConsume( + mockEnv.DATABASE, + 'single-use-registered', + new Date(Date.now() + 60_000).toISOString() + ); + mockKV.get.mockResolvedValueOnce(data); + + const first = await redeemBootstrapToken( + mockKV as unknown as KVNamespace, + 'single-use-registered', + mockEnv + ); + const second = await redeemBootstrapToken( + mockKV as unknown as KVNamespace, + 'single-use-registered', + mockEnv + ); + + expect(first?.workspaceId).toBe('ws-once'); + expect(second).toBeNull(); + expect(mockKV.get).toHaveBeenCalledTimes(1); + }); + + it('fails closed for expired registered tokens without reading KV', async () => { + const { redeemBootstrapToken, registerBootstrapTokenConsume } = await import( + '../../../src/services/bootstrap' + ); + + await registerBootstrapTokenConsume( + mockEnv.DATABASE, + 'expired-registered', + new Date(Date.now() - 1_000).toISOString() + ); + + const result = await redeemBootstrapToken( + mockKV as unknown as KVNamespace, + 'expired-registered', + mockEnv + ); + + expect(result).toBeNull(); + expect(mockKV.get).not.toHaveBeenCalled(); + expect(mockKV.delete).not.toHaveBeenCalled(); + }); + + it('allows one KV-only legacy token redemption through atomic insert-wins claim', async () => { + const { redeemBootstrapToken } = await import('../../../src/services/bootstrap'); + + const data: BootstrapTokenData = { + workspaceId: 'ws-legacy', + encryptedHetznerToken: 'encrypted-hetzner', + hetznerTokenIv: 'hetzner-iv', + callbackToken: 'legacy-callback-token', + encryptedGithubToken: null, + githubTokenIv: null, + createdAt: new Date().toISOString(), + }; + mockKV.get.mockResolvedValue(data); + + const results = await Promise.all([ + redeemBootstrapToken(mockKV as unknown as KVNamespace, 'legacy-kv-only', mockEnv), + redeemBootstrapToken(mockKV as unknown as KVNamespace, 'legacy-kv-only', mockEnv), + ]); + + expect(results.filter(Boolean)).toHaveLength(1); + expect(results.find(Boolean)?.callbackToken).toBe('legacy-callback-token'); + expect(mockKV.delete).toHaveBeenCalledTimes(1); + + const rows = sqlite.prepare('SELECT * FROM bootstrap_token_consumes WHERE token_hash = ?').all( + await tokenHash('legacy-kv-only') + ); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ consumed_at: expect.any(String) }); + }); }); describe('Token expiry (KV TTL)', () => { diff --git a/tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md b/tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md index 26fc124c0e..28c521726c 100644 --- a/tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md +++ b/tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md @@ -17,13 +17,13 @@ Bootstrap token redemption currently depends on KV `get` then `delete`, plus an ## Checklist -- [ ] Add a D1 migration for a bootstrap token consume ledger keyed by a non-secret token hash. -- [ ] Register newly created bootstrap tokens in the ledger while preserving the existing KV payload format. -- [ ] Redeem tokens only after an atomic D1 consume succeeds. -- [ ] Add migration-safe handling for in-flight legacy KV-only tokens, using atomic insert-wins semantics. -- [ ] Fail closed if the consume state is ambiguous or failed. -- [ ] Update direct legacy bootstrap-token creation to register the ledger. -- [ ] Add tests for concurrent redemption, single-use semantics, expired/missing tokens, and existing valid-token compatibility. +- [x] Add a D1 migration for a bootstrap token consume ledger keyed by a non-secret token hash. +- [x] Register newly created bootstrap tokens in the ledger while preserving the existing KV payload format. +- [x] Redeem tokens only after an atomic D1 consume succeeds. +- [x] Add migration-safe handling for in-flight legacy KV-only tokens, using atomic insert-wins semantics. +- [x] Fail closed if the consume state is ambiguous or failed. +- [x] Update direct legacy bootstrap-token creation to register the ledger. +- [x] Add tests for concurrent redemption, single-use semantics, expired/missing tokens, and existing valid-token compatibility. - [ ] Run relevant validation and local specialist reviews. - [ ] Open a narrow PR and do not merge. From 469ea02b9221c960bd19b299bc4c0dafee32353a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:45:06 +0000 Subject: [PATCH 09/35] test: cover bootstrap callback ledger path --- .../bootstrap-callback-encryption.test.ts | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts b/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts index 17260dffa2..f977d9fd8c 100644 --- a/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts +++ b/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts @@ -5,8 +5,11 @@ * to verify encrypted callbackToken decryption works end-to-end. */ import type { BootstrapResponse, BootstrapTokenData } from '@simple-agent-manager/shared'; +import Database from 'better-sqlite3'; import { Hono } from 'hono'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createSqliteD1 } from '../../helpers/sqlite-d1'; vi.mock('../../../src/middleware/rate-limit', () => { const continueRequest = async (_c: unknown, next: () => Promise) => next(); @@ -25,14 +28,28 @@ type KvMock = { const TEST_ENCRYPTION_KEY = 'iZEI8rg5FHtTo2yvt6Qw3m4z6aTfqj5MdLEGqOvdqw0='; let kv: KvMock; +let sqlite: Database.Database; let env: { KV: KvMock; - DATABASE: Record; + DATABASE: D1Database; ENCRYPTION_KEY: string; BASE_DOMAIN: string; }; +function installBootstrapLedger(db: Database.Database): void { + db.exec(` + CREATE TABLE bootstrap_token_consumes ( + token_hash TEXT PRIMARY KEY NOT NULL, + created_at TEXT NOT NULL, + expires_at TEXT NOT NULL, + consumed_at TEXT + ); + `); +} + function resetBootstrapHarness() { + sqlite = new Database(':memory:'); + installBootstrapLedger(sqlite); kv = { put: vi.fn(), get: vi.fn(), @@ -40,7 +57,7 @@ function resetBootstrapHarness() { }; env = { KV: kv, - DATABASE: {}, + DATABASE: createSqliteD1(sqlite), ENCRYPTION_KEY: TEST_ENCRYPTION_KEY, BASE_DOMAIN: 'workspaces.example.com', }; @@ -55,10 +72,14 @@ async function requestBootstrapToken(token: string) { describe('Bootstrap Callback Token Encryption (F-004)', () => { beforeEach(() => { - vi.clearAllMocks(); + vi.resetAllMocks(); resetBootstrapHarness(); }); + afterEach(() => { + sqlite.close(); + }); + it('decrypts encryptedCallbackToken via the bootstrap route', async () => { const { encrypt } = await import('../../../src/services/encryption'); From 17894bd7bfcb1d4440ba92719af74ea10df0737f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:03:48 +0000 Subject: [PATCH 10/35] docs: reconcile worker secret inventory --- apps/api/wrangler.toml | 10 +- .../docs/docs/architecture/security.md | 8 +- .../content/docs/docs/guides/self-hosting.mdx | 2 +- scripts/quality/check-wrangler-bindings.ts | 102 ++++++++++++++++-- ...026-07-29-worker-secret-inventory-drift.md | 30 ++++++ 5 files changed, 139 insertions(+), 13 deletions(-) create mode 100644 tasks/active/2026-07-29-worker-secret-inventory-drift.md diff --git a/apps/api/wrangler.toml b/apps/api/wrangler.toml index 154935c7f0..c4a495657d 100644 --- a/apps/api/wrangler.toml +++ b/apps/api/wrangler.toml @@ -356,6 +356,7 @@ crons = ["*/5 * * * *", "30 * * * *", "0 3 * * *", "0 4 * * *", "0 5 1 * *"] # - GITHUB_CLIENT_SECRET # - GITHUB_APP_ID # - GITHUB_APP_PRIVATE_KEY +# - GITHUB_APP_SLUG # - CF_API_TOKEN # - CF_ZONE_ID # - CF_ACCOUNT_ID (required for admin observability log viewer) @@ -365,8 +366,10 @@ crons = ["*/5 * * * *", "30 * * * *", "0 3 * * *", "0 4 * * *", "0 5 1 * *"] # - BETTER_AUTH_SECRET (optional — overrides ENCRYPTION_KEY for BetterAuth sessions) # - CREDENTIAL_ENCRYPTION_KEY (optional — overrides ENCRYPTION_KEY for AES-GCM credential encryption) # - GITHUB_WEBHOOK_SECRET (optional — overrides ENCRYPTION_KEY for GitHub webhook HMAC) -# - ORIGIN_CA_CERT (Origin CA certificate for VM agent TLS) -# - ORIGIN_CA_KEY (Origin CA private key for VM agent TLS) +# - DEPLOY_SIGNING_PRIVATE_KEY (Ed25519 signing key for deployment apply payloads) +# - DEPLOY_SIGNING_PUBLIC_KEY (Ed25519 verification key for deployment apply payloads) +# - DEVCONTAINER_CACHE_CLOUDFLARE_API_TOKEN (optional — narrower token for managed devcontainer registry credentials) +# - DEVCONTAINER_CACHE_CLOUDFLARE_ACCOUNT_ID (optional — account override for managed devcontainer registry credentials) # - GOOGLE_CLIENT_ID (optional — Google Cloud Console OAuth client ID for infra/GCP OIDC integration) # - GOOGLE_CLIENT_SECRET (optional — Google Cloud Console OAuth client secret for infra/GCP) # - GOOGLE_LOGIN_CLIENT_ID (optional — Google login OAuth client ID for "Sign in with Google"; separate client, or set via /setup) @@ -376,7 +379,10 @@ crons = ["*/5 * * * *", "30 * * * *", "0 3 * * *", "0 4 * * *", "0 5 1 * *"] # - GITLAB_CLIENT_SECRET (optional — GitLab OAuth secret; or set via /setup) # - SEGMENT_WRITE_KEY (optional — Segment.io write key, enables Segment event forwarding) # - GA4_API_SECRET (optional — Google Analytics 4 API secret, enables GA4 forwarding; NOTE: GA4 Measurement Protocol requires api_secret as a query parameter — ensure outbound request URLs are not logged) +# - GA4_MEASUREMENT_ID (optional — Google Analytics 4 measurement ID, paired with GA4_API_SECRET) # - R2_ACCESS_KEY_ID (optional — R2 S3-compatible API token key ID, enables task attachment presigned uploads) # - R2_SECRET_ACCESS_KEY (optional — R2 S3-compatible API token secret, enables task attachment presigned uploads) # - TRIAL_CLAIM_TOKEN_SECRET (required when trials are enabled — HMAC secret for sam_trial_claim / sam_trial_fingerprint cookies. 32+ bytes base64.) # - CF_AIG_TOKEN (optional — Cloudflare AI Gateway Unified Billing token; enables OpenAI/Anthropic models without separate API keys) +# - ANTHROPIC_API_KEY_TRIAL (optional — Anthropic API key for trial traffic when not using Workers AI) +# - SMOKE_TEST_AUTH_ENABLED (optional — enables smoke-test token auth for test environments) diff --git a/apps/www/src/content/docs/docs/architecture/security.md b/apps/www/src/content/docs/docs/architecture/security.md index e3386f614a..94454e4d68 100644 --- a/apps/www/src/content/docs/docs/architecture/security.md +++ b/apps/www/src/content/docs/docs/architecture/security.md @@ -15,7 +15,7 @@ However, SAM's own hosted deployment also has an **enabled platform-level cloud ### Platform Secrets -These are Cloudflare Worker secrets set during deployment: +These Cloudflare Worker secrets are generated or copied during deployment and are required for a fully functional install: | Secret | Purpose | | ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | @@ -28,8 +28,12 @@ These are Cloudflare Worker secrets set during deployment: | `DEPLOY_SIGNING_PUBLIC_KEY` | Ed25519 key for deployment-node payload verification (auto-generated) | | `TRIAL_CLAIM_TOKEN_SECRET` | HMAC secret for trial onboarding claim tokens (auto-generated) | | `CF_API_TOKEN` | Cloudflare deploy, DNS, Origin CA certificate issuance, observability, and AI Gateway operations (requires Account → SSL and Certificates → Edit) | +| `CF_ACCOUNT_ID` | Cloudflare account identifier used by account-scoped Cloudflare APIs | +| `CF_ZONE_ID` | Cloudflare zone identifier used for DNS and Origin CA operations | -Security keys are automatically generated and persisted by Pulumi on first deployment. Cloudflare secrets remain Worker secrets because they are deployment trust roots. GitHub App/OAuth, GitHub webhook, Google OAuth, and GitLab OAuth credentials can be supplied either as optional environment fallbacks or through the first-run/superadmin platform config UI; runtime values are stored encrypted in D1 and override environment fallbacks. They never appear in source control. +Security keys are automatically generated and persisted by Pulumi on first deployment. Cloudflare secrets remain Worker secrets because they are deployment trust roots. GitHub App/OAuth, GitHub webhook, Google OAuth, GitLab OAuth, analytics forwarding, R2 attachment-upload credentials, devcontainer cache credentials, trial provider keys, and smoke-test auth flags can be supplied as optional Worker secret fallbacks when an installation needs them. Runtime platform values saved through first-run setup or the superadmin platform config UI are stored encrypted in D1 and override environment fallbacks. They never appear in source control. + +New VM nodes do not require static `ORIGIN_CA_CERT` or `ORIGIN_CA_KEY` Worker secrets. If those legacy secrets exist from an older deployment, remove them after draining old nodes and confirming the per-node CSR model is deployed. ### Platform Integration Credentials diff --git a/apps/www/src/content/docs/docs/guides/self-hosting.mdx b/apps/www/src/content/docs/docs/guides/self-hosting.mdx index 6474f2d5c4..03a3625cb0 100644 --- a/apps/www/src/content/docs/docs/guides/self-hosting.mdx +++ b/apps/www/src/content/docs/docs/guides/self-hosting.mdx @@ -176,7 +176,7 @@ without their own cloud credential, on [Instant sessions](/docs/guides/instant-s Setting it to `false` means every session provisions a cloud VM, so **each user must connect their own cloud provider credential before they can do anything**. -**Environment secrets:** +**GitHub Environment secrets:** | Secret | Description | | ------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | diff --git a/scripts/quality/check-wrangler-bindings.ts b/scripts/quality/check-wrangler-bindings.ts index 6b8409956b..c848930605 100644 --- a/scripts/quality/check-wrangler-bindings.ts +++ b/scripts/quality/check-wrangler-bindings.ts @@ -11,6 +11,9 @@ * generated env sections. If they're missing at the top level, they'll be * missing at runtime. * + * 3. Worker secret inventory comments stay aligned with configure-secrets.sh + * so operator-facing docs/config comments do not silently drift. + * * This check runs in CI to prevent misconfigurations. */ import { readFileSync } from 'node:fs'; @@ -18,7 +21,11 @@ import { resolve } from 'node:path'; import * as TOML from '@iarna/toml'; const API_WRANGLER_PATH = resolve(import.meta.dirname, '../../apps/api/wrangler.toml'); -const TAIL_WORKER_WRANGLER_PATH = resolve(import.meta.dirname, '../../apps/tail-worker/wrangler.toml'); +const TAIL_WORKER_WRANGLER_PATH = resolve( + import.meta.dirname, + '../../apps/tail-worker/wrangler.toml' +); +const CONFIGURE_SECRETS_PATH = resolve(import.meta.dirname, '../deploy/configure-secrets.sh'); interface Binding { name?: string; @@ -55,6 +62,48 @@ function fail(errors: string[]): never { process.exit(1); } +function extractConfiguredWorkerSecrets(scriptContent: string): string[] { + return Array.from( + scriptContent.matchAll(/set_worker_secret\s+"([A-Z0-9_]+)"/g), + (match) => match[1] + ) + .filter((name, index, all) => all.indexOf(name) === index) + .sort(); +} + +function extractWranglerCommentedSecrets(wranglerContent: string): string[] { + const secretsHeader = 'Secrets (set via wrangler secret put):'; + const start = wranglerContent.indexOf(secretsHeader); + if (start === -1) { + return []; + } + + const afterHeader = wranglerContent.slice(wranglerContent.indexOf('\n', start) + 1); + const lines = afterHeader.split('\n'); + const secrets: string[] = []; + + for (const line of lines) { + if (!line.startsWith('#')) { + break; + } + const match = line.match(/^# - `?([A-Z0-9_]+)`?/); + if (match) { + secrets.push(match[1]); + } + } + + return secrets.filter((name, index, all) => all.indexOf(name) === index).sort(); +} + +function diffSecrets(expected: string[], actual: string[]): { missing: string[]; extra: string[] } { + const actualSet = new Set(actual); + const expectedSet = new Set(expected); + return { + missing: expected.filter((name) => !actualSet.has(name)), + extra: actual.filter((name) => !expectedSet.has(name)), + }; +} + function main(): void { const errors: string[] = []; @@ -64,13 +113,14 @@ function main(): void { const apiContent = readFileSync(API_WRANGLER_PATH, 'utf-8'); const apiConfig = TOML.parse(apiContent) as unknown as WranglerConfig; + const configureSecretsContent = readFileSync(CONFIGURE_SECRETS_PATH, 'utf-8'); if (apiConfig.env && Object.keys(apiConfig.env).length > 0) { const envNames = Object.keys(apiConfig.env).join(', '); errors.push( `apps/api/wrangler.toml contains [env.*] sections (${envNames}). ` + - `These are generated at deploy time by sync-wrangler-config.ts. ` + - `Remove them from the checked-in file.` + `These are generated at deploy time by sync-wrangler-config.ts. ` + + `Remove them from the checked-in file.` ); } @@ -81,7 +131,7 @@ function main(): void { const envNames = Object.keys(tailConfig.env).join(', '); errors.push( `apps/tail-worker/wrangler.toml contains [env.*] sections (${envNames}). ` + - `These are generated at deploy time. Remove them from the checked-in file.` + `These are generated at deploy time. Remove them from the checked-in file.` ); } @@ -90,11 +140,15 @@ function main(): void { // ======================================== if (!apiConfig.durable_objects?.bindings?.length) { - errors.push('apps/api/wrangler.toml: top-level missing durable_objects.bindings (sync script copies these to env sections)'); + errors.push( + 'apps/api/wrangler.toml: top-level missing durable_objects.bindings (sync script copies these to env sections)' + ); } if (!apiConfig.ai?.binding) { - errors.push('apps/api/wrangler.toml: top-level missing [ai] binding (sync script copies this to env sections)'); + errors.push( + 'apps/api/wrangler.toml: top-level missing [ai] binding (sync script copies this to env sections)' + ); } if (!apiConfig.d1_databases?.length) { @@ -110,7 +164,34 @@ function main(): void { } if (!apiConfig.migrations?.length) { - errors.push('apps/api/wrangler.toml: top-level missing [[migrations]] (sync script copies these to env sections)'); + errors.push( + 'apps/api/wrangler.toml: top-level missing [[migrations]] (sync script copies these to env sections)' + ); + } + + // ======================================== + // Check 3: Worker secret inventory comment matches configure-secrets.sh + // ======================================== + + const configuredSecrets = extractConfiguredWorkerSecrets(configureSecretsContent); + const commentedSecrets = extractWranglerCommentedSecrets(apiContent); + + if (commentedSecrets.length === 0) { + errors.push( + 'apps/api/wrangler.toml: missing "# Secrets (set via wrangler secret put):" inventory comment' + ); + } else { + const { missing, extra } = diffSecrets(configuredSecrets, commentedSecrets); + if (missing.length > 0) { + errors.push( + `apps/api/wrangler.toml secret inventory is missing configured secrets from configure-secrets.sh: ${missing.join(', ')}` + ); + } + if (extra.length > 0) { + errors.push( + `apps/api/wrangler.toml secret inventory lists secrets not configured by configure-secrets.sh: ${extra.join(', ')}` + ); + } } // ======================================== @@ -129,7 +210,12 @@ function main(): void { console.log('Wrangler config check passed.'); console.log(` No [env.*] sections in checked-in files.`); - console.log(` Top-level: ${doCount} DOs, ${d1Count} D1, ${kvCount} KV, ${r2Count} R2, AI, ${migrationCount} migrations`); + console.log( + ` Top-level: ${doCount} DOs, ${d1Count} D1, ${kvCount} KV, ${r2Count} R2, AI, ${migrationCount} migrations` + ); + console.log( + ` Worker secret inventory: ${configuredSecrets.length} configured secrets documented.` + ); } main(); diff --git a/tasks/active/2026-07-29-worker-secret-inventory-drift.md b/tasks/active/2026-07-29-worker-secret-inventory-drift.md new file mode 100644 index 0000000000..5289f1259b --- /dev/null +++ b/tasks/active/2026-07-29-worker-secret-inventory-drift.md @@ -0,0 +1,30 @@ +# Reconcile Worker secret inventory docs + +## Problem + +The public deployment/self-hosting docs and checked-in Worker secret inventory comments have drifted from the deploy script that actually writes Worker secrets. In particular, `apps/api/wrangler.toml` still listed legacy Origin CA Worker secrets while omitting newer deployment signing and optional fallback secrets. + +## Research findings + +- Public operator docs live under `apps/www/src/content/docs/docs/`; non-public Markdown should not be used as user-facing documentation. +- `scripts/deploy/configure-secrets.sh` is the operational source for Worker secrets written during deployment. +- `scripts/deploy/types.ts` declares required and optional deploy secret categories but is not itself a complete list of every optional Worker secret written by the shell script. +- `apps/api/wrangler.toml` contains the visible checked-in Worker secret inventory comment. +- `scripts/quality/check-wrangler-bindings.ts` already runs in CI via `pnpm quality:wrangler-bindings`, making it the lowest-risk place to add an inventory drift check. +- New VM nodes use per-node CSR/Origin CA issuance and do not require static `ORIGIN_CA_CERT` or `ORIGIN_CA_KEY` Worker secrets. + +## Checklist + +- [x] Update the `wrangler.toml` Worker secret inventory comment to match configured secrets. +- [x] Update public self-hosting/security docs to clarify generated Worker secrets, optional fallback secrets, and legacy Origin CA cleanup. +- [x] Add a quality check that compares `configure-secrets.sh` `set_worker_secret` calls with the `wrangler.toml` inventory comment. +- [ ] Run targeted checks. +- [ ] Run doc-sync and env-validator reviews. +- [ ] Open PR and wait for CI without merging. + +## Acceptance criteria + +- Public docs no longer imply legacy Origin CA Worker secrets are required. +- Checked-in Worker secret inventory cannot drift silently from `configure-secrets.sh`. +- No runtime behavior changes. +- PR is open, CI is green, and remains unmerged. From f57e099fb74e3be717f8599090fbc1a0bd5fccb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:53:34 +0000 Subject: [PATCH 11/35] task: add vm-agent shutdown idempotency --- ...026-07-29-vm-agent-shutdown-idempotency.md | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tasks/backlog/2026-07-29-vm-agent-shutdown-idempotency.md diff --git a/tasks/backlog/2026-07-29-vm-agent-shutdown-idempotency.md b/tasks/backlog/2026-07-29-vm-agent-shutdown-idempotency.md new file mode 100644 index 0000000000..09048f2f54 --- /dev/null +++ b/tasks/backlog/2026-07-29-vm-agent-shutdown-idempotency.md @@ -0,0 +1,44 @@ +# VM-agent shutdown idempotency + +## Problem + +VM-agent shutdown paths are not fully idempotent. `auth.SessionManager.Stop()` closes its cleanup channel directly, so repeated calls panic. `server.Server.Stop()` also closes its `done` channel directly and does not stop the owned auth session cleanup goroutine. Shutdown may therefore panic during repeated lifecycle cleanup and may leave an owned goroutine running. + +This task is a retry of the failed startup task `01KYQJ1P3FPQBDPCWFQ0SWPAYJ`; duplicate check found no active queued duplicate for VM-agent shutdown idempotency. + +## Research findings + +- `packages/vm-agent/internal/auth/session.go` + - `NewSessionManagerWithConfig()` starts a cleanup goroutine. + - `SessionManager.Stop()` currently calls `close(sm.stopCleanup)` directly, making repeated calls panic. + - Tests in `session_test.go` defer `sm.Stop()` in many cases, so the public cleanup API is already expected to be safe to call from tests and shutdown paths. +- `packages/vm-agent/internal/server/server.go` + - `Server.Start()` starts node health, ACP heartbeat, and error reporter background work. + - `Server.Stop()` closes `s.done` directly, so repeated calls can panic. + - `Server.Stop()` stops port scanners, JWT validator, ACP session hosts, PTY sessions, reporters, persistence, and HTTP server, but does not stop `s.sessionManager`. +- Relevant rules: + - `.claude/rules/46-vm-agent-diagnostic-getter-sync.md`: vm-agent background goroutine state must be synchronized and race-tested where feasible. + - `.claude/rules/02-quality-gates.md`: lifecycle bug fixes need regression tests that would have caught the violated invariant. + - `.claude/rules/14-do-workflow-persistence.md` and `.claude/rules/25-review-merge-gate.md`: maintain `.do-state.md` and complete specialist reviews before PR completion. + +## Implementation checklist + +- [ ] Make `auth.SessionManager.Stop()` idempotent and safe under repeated/concurrent calls without changing public API shape. +- [ ] Ensure `auth.SessionManager` cleanup goroutine exits when `Stop()` is called. +- [ ] Make `server.Server.Stop()` idempotent for repeated/concurrent calls without changing public API shape. +- [ ] Ensure `Server.Stop()` stops the owned auth session cleanup goroutine. +- [ ] Preserve current shutdown ordering for external behavior unless a narrow ordering change is required for complete cleanup. +- [ ] Add focused Go tests for repeated and concurrent `SessionManager.Stop()`. +- [ ] Add focused Go tests proving `Server.Stop()` calls auth session cleanup and repeated `Server.Stop()` does not panic. +- [ ] Run relevant Go tests, including `-race` if feasible. +- [ ] Run local `go-specialist`, `test-engineer`, and task-completion validation reviews and address findings. +- [ ] Open a PR against `main`, wait for CI, and do not merge. + +## Acceptance criteria + +- Repeated `SessionManager.Stop()` calls do not panic. +- Repeated `Server.Stop()` calls do not panic. +- `Server.Stop()` cleanly stops owned background goroutines, including auth session cleanup. +- No external/public API changes are introduced. +- Strong Go regression tests cover the shutdown idempotency contract and pass locally. +- PR includes specialist review evidence, tests run, CI status, and a no-breaking-change rationale. From 26d59051705e4be0717ca77f71f5da05584f709e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:40:24 +0000 Subject: [PATCH 12/35] task: add configurable container max instances --- ...29-configurable-container-max-instances.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 tasks/backlog/2026-07-29-configurable-container-max-instances.md diff --git a/tasks/backlog/2026-07-29-configurable-container-max-instances.md b/tasks/backlog/2026-07-29-configurable-container-max-instances.md new file mode 100644 index 0000000000..9e622d16c8 --- /dev/null +++ b/tasks/backlog/2026-07-29-configurable-container-max-instances.md @@ -0,0 +1,47 @@ +# Make Cloudflare container max_instances configurable + +## Problem statement + +Cloudflare container `max_instances` is checked into `apps/api/wrangler.toml` as static values for the sandbox and VM-agent container bindings. A prior startup task failed before agent startup with a Hetzner 422 unrelated to this code path, and the requested retry is a tightly scoped remediation PR: keep the current safe defaults exactly unchanged while making the Cloudflare container capacity limits configurable through generic deployment configuration. + +## Research findings + +- `apps/api/wrangler.toml` defines two top-level `[[containers]]` blocks: + - `SandboxDO` with `max_instances = 6` + - `VmAgentContainer` with `max_instances = 3` +- `scripts/deploy/sync-wrangler-config.ts` copies top-level `containers` into generated `[env.*]` blocks through `extractStaticBindings()` and `getStaticApiWorkerBindings()`. +- `scripts/quality/sync-wrangler-config.test.ts` already tests generated Worker vars and deployment-time override pass-through for cf-container tunables. +- `.github/workflows/deploy-reusable.yml` forwards optional GitHub Environment vars into the Wrangler sync step; new max-instance overrides must be forwarded there too. +- `.claude/rules/43-long-running-mcp-tools.md` and `tasks/archive/2026-07-19-fix-instant-container-clone-timeout.md` document a previous cf-container deploy plumbing issue where deployment tunables needed explicit workflow and sync-script coverage. +- `packages/shared/AGENTS.md` notes configurable defaults should follow env-var resolution patterns and validation should use Valibot. + +## Implementation checklist + +- [ ] Add centralized container max-instance config in the Wrangler sync script with defaults exactly `SandboxDO=6` and `VmAgentContainer=3`. +- [ ] Add generic deployment environment variable names for overriding those limits. +- [ ] Validate overrides as positive safe integers before generating Wrangler config. +- [ ] Generate/sync container blocks from centralized config instead of copying static `max_instances` through unchanged. +- [ ] Forward the new optional variables from `.github/workflows/deploy-reusable.yml`. +- [ ] Add quality tests proving defaults remain `6` and `3`. +- [ ] Add quality tests proving overrides are respected. +- [ ] Add quality tests proving invalid overrides fail closed. +- [ ] Run local Cloudflare/config/test specialist reviews and address findings. +- [ ] Open a PR against `main`, wait for CI, and do not merge. + +## Acceptance criteria + +- Generated `containers` bindings preserve the existing defaults exactly: `SandboxDO.max_instances = 6`, `VmAgentContainer.max_instances = 3`. +- Operators can override each container limit through generic deployment configuration without editing checked-in wrangler files. +- Invalid override values stop config generation with a clear error. +- Deployment workflow forwards the new variables to the sync script. +- Tests cover defaults, valid overrides, invalid overrides, and workflow forwarding. +- PR is open, CI is green, and the PR is not merged. + +## References + +- `apps/api/wrangler.toml` +- `scripts/deploy/sync-wrangler-config.ts` +- `scripts/quality/sync-wrangler-config.test.ts` +- `.github/workflows/deploy-reusable.yml` +- `.claude/rules/43-long-running-mcp-tools.md` +- `tasks/archive/2026-07-19-fix-instant-container-clone-timeout.md` From 0f1b825b47f90ef44b2bc3d88328011ea0cdb45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:54:18 +0000 Subject: [PATCH 13/35] fix: use integer bootstrap consume timestamps --- .../0101_bootstrap_token_consumes.sql | 6 ++--- apps/api/src/services/bootstrap.ts | 22 ++++++++++++++----- apps/api/tests/unit/routes/bootstrap.test.ts | 6 ++--- .../bootstrap-callback-encryption.test.ts | 6 ++--- .../api/tests/unit/services/bootstrap.test.ts | 8 +++---- 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql b/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql index f0433906f4..b0b5517835 100644 --- a/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql +++ b/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql @@ -1,8 +1,8 @@ CREATE TABLE IF NOT EXISTS bootstrap_token_consumes ( token_hash TEXT PRIMARY KEY NOT NULL, - created_at TEXT NOT NULL, - expires_at TEXT NOT NULL, - consumed_at TEXT + created_at INTEGER NOT NULL, + expires_at INTEGER NOT NULL, + consumed_at INTEGER ); CREATE INDEX IF NOT EXISTS idx_bootstrap_token_consumes_expiry ON bootstrap_token_consumes(expires_at); CREATE INDEX IF NOT EXISTS idx_bootstrap_token_consumes_consumed ON bootstrap_token_consumes(consumed_at); diff --git a/apps/api/src/services/bootstrap.ts b/apps/api/src/services/bootstrap.ts index 78f6b5f1f7..521a96b196 100644 --- a/apps/api/src/services/bootstrap.ts +++ b/apps/api/src/services/bootstrap.ts @@ -161,7 +161,7 @@ export async function registerBootstrapTokenConsume( `INSERT INTO bootstrap_token_consumes (token_hash, created_at, expires_at) VALUES (?, ?, ?)` ) - .bind(await hashBootstrapToken(token), new Date().toISOString(), expiresAt) + .bind(await hashBootstrapToken(token), Date.now(), parseBootstrapExpiry(expiresAt)) .run(); } @@ -171,7 +171,7 @@ async function reserveBootstrapTokenConsume( db: D1Database, token: string ): Promise { - const now = new Date().toISOString(); + const now = Date.now(); const tokenHash = await hashBootstrapToken(token); const updated = await db @@ -202,7 +202,7 @@ async function claimLegacyBootstrapTokenConsume( token: string, env: Pick ): Promise { - const now = new Date().toISOString(); + const now = Date.now(); // Migration-safe compatibility for unexpired tokens that were written to KV // before the D1 ledger existed, or direct legacy producers that only wrote KV. @@ -213,7 +213,7 @@ async function claimLegacyBootstrapTokenConsume( `INSERT OR IGNORE INTO bootstrap_token_consumes (token_hash, created_at, expires_at, consumed_at) VALUES (?, ?, ?, ?)` ) - .bind(await hashBootstrapToken(token), now, nowPlusSeconds(getBootstrapTTL(env)), now) + .bind(await hashBootstrapToken(token), now, nowPlusSecondsMs(getBootstrapTTL(env)), now) .run(); return d1Changes(legacyClaim) === 1; @@ -232,7 +232,19 @@ async function hashBootstrapToken(token: string): Promise { } function nowPlusSeconds(seconds: number): string { - return new Date(Date.now() + seconds * 1000).toISOString(); + return new Date(nowPlusSecondsMs(seconds)).toISOString(); +} + +function nowPlusSecondsMs(seconds: number): number { + return Date.now() + seconds * 1000; +} + +function parseBootstrapExpiry(expiresAt: string): number { + const parsed = Date.parse(expiresAt); + if (!Number.isFinite(parsed)) { + throw new Error('Invalid bootstrap token expiry'); + } + return parsed; } function isLegacyPlaintextCallbackTokenStillRedeemable( diff --git a/apps/api/tests/unit/routes/bootstrap.test.ts b/apps/api/tests/unit/routes/bootstrap.test.ts index e8799235fd..09f70f2024 100644 --- a/apps/api/tests/unit/routes/bootstrap.test.ts +++ b/apps/api/tests/unit/routes/bootstrap.test.ts @@ -32,9 +32,9 @@ function installBootstrapLedger(db: Database.Database): void { db.exec(` CREATE TABLE bootstrap_token_consumes ( token_hash TEXT PRIMARY KEY NOT NULL, - created_at TEXT NOT NULL, - expires_at TEXT NOT NULL, - consumed_at TEXT + created_at INTEGER NOT NULL, + expires_at INTEGER NOT NULL, + consumed_at INTEGER ); `); } diff --git a/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts b/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts index f977d9fd8c..da4d94468f 100644 --- a/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts +++ b/apps/api/tests/unit/services/bootstrap-callback-encryption.test.ts @@ -40,9 +40,9 @@ function installBootstrapLedger(db: Database.Database): void { db.exec(` CREATE TABLE bootstrap_token_consumes ( token_hash TEXT PRIMARY KEY NOT NULL, - created_at TEXT NOT NULL, - expires_at TEXT NOT NULL, - consumed_at TEXT + created_at INTEGER NOT NULL, + expires_at INTEGER NOT NULL, + consumed_at INTEGER ); `); } diff --git a/apps/api/tests/unit/services/bootstrap.test.ts b/apps/api/tests/unit/services/bootstrap.test.ts index 2695384b84..6eb0156303 100644 --- a/apps/api/tests/unit/services/bootstrap.test.ts +++ b/apps/api/tests/unit/services/bootstrap.test.ts @@ -22,9 +22,9 @@ function installBootstrapLedger(db: Database.Database): void { db.exec(` CREATE TABLE bootstrap_token_consumes ( token_hash TEXT PRIMARY KEY NOT NULL, - created_at TEXT NOT NULL, - expires_at TEXT NOT NULL, - consumed_at TEXT + created_at INTEGER NOT NULL, + expires_at INTEGER NOT NULL, + consumed_at INTEGER ); `); } @@ -294,7 +294,7 @@ describe('Bootstrap Service', () => { await tokenHash('legacy-kv-only') ); expect(rows).toHaveLength(1); - expect(rows[0]).toMatchObject({ consumed_at: expect.any(String) }); + expect(rows[0]).toMatchObject({ consumed_at: expect.any(Number) }); }); }); From b1fb1599f5cd90471db64b742d2744bef7638259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 20:03:21 +0000 Subject: [PATCH 14/35] task: activate strict CTO remediation integration --- ...26-07-29-strict-cto-remediation-mega-pr.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md diff --git a/tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md b/tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md new file mode 100644 index 0000000000..648632fc04 --- /dev/null +++ b/tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md @@ -0,0 +1,50 @@ +# Strict CTO remediation mega PR + +## Problem + +Eight remediation PRs have been completed independently and need to be integrated into one safe mega PR. The integration must preserve each remediation's tests and guarantees, validate the combined diff locally and on staging, and merge to production only after all gates pass. + +## Input PRs + +- #1689 default-branch/output-branch safety +- #1690 deploy-reusable Wrangler sync env parity +- #1691 fail-closed workers.dev/cron setup +- #1692 deployment docs and Worker secret inventory drift +- #1693 VM-agent shutdown idempotency +- #1694 configurable Cloudflare container max_instances +- #1695 ProjectData row fault isolation + bootstrap TTL wording +- #1696 atomic bootstrap token redemption + +## Research findings + +- SAM instructions require progress updates, use of the output branch `sam/execute-task-using-skill-ggdn3n`, and no merge until validation is complete. +- Existing local main worktree has an unrelated `.codex/config.toml` modification that must be preserved and excluded. +- All eight PRs are open against `main` and initially report clean merge state from GitHub. +- Affected areas are expected to include task dispatch/branch handling, deployment scripts/docs/env inventory, Cloudflare Worker setup, VM agent Go shutdown behavior, ProjectData Durable Object message listing, and bootstrap token redemption. + +## Checklist + +- [ ] Create integration branch from current `origin/main`. +- [ ] Merge/cherry-pick PRs #1689 through #1696. +- [ ] Resolve conflicts without dropping tests or docs from any remediation. +- [ ] Run targeted tests for all affected areas. +- [ ] Run full feasible local validation: lint, typecheck, tests, build. +- [ ] Run local specialist reviews for correctness, security, Cloudflare/env consistency, Go quality, task completion, and test quality. +- [ ] Open mega PR with specialist review evidence. +- [ ] Wait for CI to be completely green. +- [ ] Check staging state before deploy and avoid clobbering active validation. +- [ ] Deploy the mega PR to staging. +- [ ] Validate affected surfaces on staging: task dispatch/output branches, bootstrap token flow, ProjectData message listing, deploy workflow config behavior, docs/build/quality checks, and VM-agent shutdown as safely testable. +- [ ] Validate core agent workflows on staging for both Claude and Codex using Playwright token-login and platform credential fallback. +- [ ] Merge mega PR only after all gates pass. +- [ ] Monitor production Deploy Production workflow by merge `headSha` and require a successful deploy run. +- [ ] Report final PR URL, merge commit, staging evidence, core Claude/Codex evidence, production deploy evidence, and source PR disposition. + +## Acceptance criteria + +- One integration PR contains all eight remediation PRs' intended code, docs, and tests. +- CI is green on the integration PR. +- Staging deploy succeeds and all listed affected surfaces are verified. +- Both Claude and Codex core staging workflows return valid responses. +- Production deploy succeeds after merge. +- No individual remediation PR is merged as a substitute for the mega PR. From 6733fb7a395b3cd04dcd5283d4fe9e29181feafd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:04:14 +0000 Subject: [PATCH 15/35] task: archive worker secret inventory drift --- .../2026-07-29-worker-secret-inventory-drift.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename tasks/{active => archive}/2026-07-29-worker-secret-inventory-drift.md (96%) diff --git a/tasks/active/2026-07-29-worker-secret-inventory-drift.md b/tasks/archive/2026-07-29-worker-secret-inventory-drift.md similarity index 96% rename from tasks/active/2026-07-29-worker-secret-inventory-drift.md rename to tasks/archive/2026-07-29-worker-secret-inventory-drift.md index 5289f1259b..da1001e7a9 100644 --- a/tasks/active/2026-07-29-worker-secret-inventory-drift.md +++ b/tasks/archive/2026-07-29-worker-secret-inventory-drift.md @@ -18,8 +18,8 @@ The public deployment/self-hosting docs and checked-in Worker secret inventory c - [x] Update the `wrangler.toml` Worker secret inventory comment to match configured secrets. - [x] Update public self-hosting/security docs to clarify generated Worker secrets, optional fallback secrets, and legacy Origin CA cleanup. - [x] Add a quality check that compares `configure-secrets.sh` `set_worker_secret` calls with the `wrangler.toml` inventory comment. -- [ ] Run targeted checks. -- [ ] Run doc-sync and env-validator reviews. +- [x] Run targeted checks. +- [x] Run doc-sync and env-validator reviews. - [ ] Open PR and wait for CI without merging. ## Acceptance criteria From 463ba424b2570f06d30a1345384baf496675fe3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:54:34 +0000 Subject: [PATCH 16/35] task: activate vm-agent shutdown idempotency --- .../2026-07-29-vm-agent-shutdown-idempotency.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tasks/{backlog => active}/2026-07-29-vm-agent-shutdown-idempotency.md (100%) diff --git a/tasks/backlog/2026-07-29-vm-agent-shutdown-idempotency.md b/tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md similarity index 100% rename from tasks/backlog/2026-07-29-vm-agent-shutdown-idempotency.md rename to tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md From 5ea856099cf77a7bc745d0863ef00182c6107c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:41:02 +0000 Subject: [PATCH 17/35] task: mark container max instances active --- .../2026-07-29-configurable-container-max-instances.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tasks/{backlog => active}/2026-07-29-configurable-container-max-instances.md (100%) diff --git a/tasks/backlog/2026-07-29-configurable-container-max-instances.md b/tasks/active/2026-07-29-configurable-container-max-instances.md similarity index 100% rename from tasks/backlog/2026-07-29-configurable-container-max-instances.md rename to tasks/active/2026-07-29-configurable-container-max-instances.md From 3ae0ab6921bb93c821bfcffae64518becdcd18f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:39:25 +0000 Subject: [PATCH 18/35] task: add project data list row isolation --- ...6-07-29-project-data-list-row-isolation.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tasks/backlog/2026-07-29-project-data-list-row-isolation.md diff --git a/tasks/backlog/2026-07-29-project-data-list-row-isolation.md b/tasks/backlog/2026-07-29-project-data-list-row-isolation.md new file mode 100644 index 0000000000..cbcd023b3d --- /dev/null +++ b/tasks/backlog/2026-07-29-project-data-list-row-isolation.md @@ -0,0 +1,36 @@ +# ProjectData list row isolation hardening + +## Problem + +ProjectData Durable Object list reads must not fail an entire API/list response when one stored row is malformed or no longer matches the current row schema. The sessions list path already has row-level isolation, but message list reads still map fetched rows through throwing parsers. A malformed `chat_messages` row can therefore break message list retrieval for otherwise valid session history. + +Also fix the stale bootstrap TTL comment/test wording so it remains correct when the TTL is configurable. + +## Research findings + +- `.claude/rules/50-list-read-row-fault-isolation.md` requires per-row parse/enrichment isolation for multi-row D1/DO SQLite reads. +- `apps/api/src/durable-objects/project-data/sessions.ts` already uses `enrichSessionRows()` to skip and warn-log malformed session rows without changing the response contract. +- `apps/api/src/durable-objects/project-data/messages.ts:getMessages()` still does `orderedRows.map(parseChatMessageRow...)`, so one malformed row throws the whole read. +- `apps/api/tests/unit/durable-objects/project-data-messages.test.ts` has focused unit coverage for `getMessages()` ordering and pagination. +- `apps/api/src/services/bootstrap.ts` and `apps/api/tests/unit/services/bootstrap.test.ts` describe the default TTL as 15 minutes/900 seconds, but comments should not imply a fixed TTL when `BOOTSTRAP_TOKEN_TTL_SECONDS` overrides it. + +## Checklist + +- [ ] Add row-level parse isolation to `getMessages()` for normal and compact message rows. +- [ ] Warn-log skipped message rows with context, best-effort row id/session id, compact mode, and parser error. +- [ ] Preserve the existing `{ messages, hasMore }` response contract and ordering behavior. +- [ ] Add a good/bad/good regression test for malformed message rows. +- [ ] Add an all-bad regression test returning an empty non-throwing list. +- [ ] Update stale bootstrap TTL comment/test wording without changing runtime behavior. +- [ ] Run targeted tests and broader validation. +- [ ] Run local reviewer/subagent checks for tests and code review. +- [ ] Open PR, wait for CI, and do not merge. + +## Acceptance criteria + +- Message list reads skip malformed rows and return valid rows instead of throwing. +- Skip behavior is diagnosable via structured warn logging. +- Existing API/response shape is unchanged. +- Targeted tests cover malformed rows among valid rows and the all-bad case. +- Bootstrap TTL comment/test wording reflects configurability. +- PR is open with CI green and remains unmerged. From 7e536e87676bbc0fdb167736bc55636367b66b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:54:30 +0000 Subject: [PATCH 19/35] task: archive atomic bootstrap token redemption --- .../2026-07-29-atomic-bootstrap-token-redemption.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename tasks/{active => archive}/2026-07-29-atomic-bootstrap-token-redemption.md (95%) diff --git a/tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md b/tasks/archive/2026-07-29-atomic-bootstrap-token-redemption.md similarity index 95% rename from tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md rename to tasks/archive/2026-07-29-atomic-bootstrap-token-redemption.md index 28c521726c..d050c860a7 100644 --- a/tasks/active/2026-07-29-atomic-bootstrap-token-redemption.md +++ b/tasks/archive/2026-07-29-atomic-bootstrap-token-redemption.md @@ -24,8 +24,8 @@ Bootstrap token redemption currently depends on KV `get` then `delete`, plus an - [x] Fail closed if the consume state is ambiguous or failed. - [x] Update direct legacy bootstrap-token creation to register the ledger. - [x] Add tests for concurrent redemption, single-use semantics, expired/missing tokens, and existing valid-token compatibility. -- [ ] Run relevant validation and local specialist reviews. -- [ ] Open a narrow PR and do not merge. +- [x] Run relevant validation and local specialist reviews. +- [x] Open a narrow PR and do not merge. ## Acceptance criteria From 1e50b5539521cc69cccd762406ca7edc3edb3d31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:09:41 +0000 Subject: [PATCH 20/35] chore: harden worker secret inventory check --- scripts/quality/check-wrangler-bindings.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/scripts/quality/check-wrangler-bindings.ts b/scripts/quality/check-wrangler-bindings.ts index c848930605..e46010fbdd 100644 --- a/scripts/quality/check-wrangler-bindings.ts +++ b/scripts/quality/check-wrangler-bindings.ts @@ -64,11 +64,10 @@ function fail(errors: string[]): never { function extractConfiguredWorkerSecrets(scriptContent: string): string[] { return Array.from( - scriptContent.matchAll(/set_worker_secret\s+"([A-Z0-9_]+)"/g), - (match) => match[1] - ) - .filter((name, index, all) => all.indexOf(name) === index) - .sort(); + new Set( + Array.from(scriptContent.matchAll(/set_worker_secret\s+"([A-Z0-9_]+)"/g), (match) => match[1]) + ) + ).sort(); } function extractWranglerCommentedSecrets(wranglerContent: string): string[] { @@ -78,7 +77,12 @@ function extractWranglerCommentedSecrets(wranglerContent: string): string[] { return []; } - const afterHeader = wranglerContent.slice(wranglerContent.indexOf('\n', start) + 1); + const headerLineEnd = wranglerContent.indexOf('\n', start); + if (headerLineEnd === -1) { + return []; + } + + const afterHeader = wranglerContent.slice(headerLineEnd + 1); const lines = afterHeader.split('\n'); const secrets: string[] = []; @@ -92,7 +96,7 @@ function extractWranglerCommentedSecrets(wranglerContent: string): string[] { } } - return secrets.filter((name, index, all) => all.indexOf(name) === index).sort(); + return Array.from(new Set(secrets)).sort(); } function diffSecrets(expected: string[], actual: string[]): { missing: string[]; extra: string[] } { From 6db3cad480605d6831133200bae12960177b25a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:01:07 +0000 Subject: [PATCH 21/35] fix: make vm-agent shutdown idempotent --- packages/vm-agent/internal/auth/session.go | 9 +- .../vm-agent/internal/auth/session_test.go | 40 ++++++ packages/vm-agent/internal/server/server.go | 74 ++++++----- .../vm-agent/internal/server/shutdown_test.go | 116 ++++++++++++++++++ ...026-07-29-vm-agent-shutdown-idempotency.md | 16 +-- 5 files changed, 216 insertions(+), 39 deletions(-) create mode 100644 packages/vm-agent/internal/server/shutdown_test.go diff --git a/packages/vm-agent/internal/auth/session.go b/packages/vm-agent/internal/auth/session.go index af3752bfbc..a2cd69833c 100644 --- a/packages/vm-agent/internal/auth/session.go +++ b/packages/vm-agent/internal/auth/session.go @@ -32,6 +32,8 @@ type SessionManager struct { maxSessions int cookieDomain string // Domain for cookie sharing across subdomains (e.g., ".example.com") stopCleanup chan struct{} + cleanupDone chan struct{} + stopOnce sync.Once } // SessionManagerConfig holds configuration for the session manager. @@ -67,6 +69,7 @@ func NewSessionManagerWithConfig(cfg SessionManagerConfig) *SessionManager { maxSessions: cfg.MaxSessions, cookieDomain: cfg.CookieDomain, stopCleanup: make(chan struct{}), + cleanupDone: make(chan struct{}), } // Start cleanup goroutine @@ -262,6 +265,7 @@ func (sm *SessionManager) ClearCookie(w http.ResponseWriter) { func (sm *SessionManager) cleanup() { ticker := time.NewTicker(sm.cleanupInterval) defer ticker.Stop() + defer close(sm.cleanupDone) for { select { @@ -288,7 +292,10 @@ func (sm *SessionManager) cleanup() { // Stop stops the cleanup goroutine. func (sm *SessionManager) Stop() { - close(sm.stopCleanup) + sm.stopOnce.Do(func() { + close(sm.stopCleanup) + }) + <-sm.cleanupDone } // generateSessionID generates a random session ID. diff --git a/packages/vm-agent/internal/auth/session_test.go b/packages/vm-agent/internal/auth/session_test.go index f175fe6818..be5e70fa13 100644 --- a/packages/vm-agent/internal/auth/session_test.go +++ b/packages/vm-agent/internal/auth/session_test.go @@ -183,3 +183,43 @@ func TestGetSessionForWorkspace_ExpiredScopedCookie(t *testing.T) { t.Error("expected to fall back to legacy cookie when scoped session is expired") } } + +func TestSessionManagerStopIsIdempotent(t *testing.T) { + sm := newTestSessionManager() + + sm.Stop() + sm.Stop() +} + +func TestSessionManagerStopIsSafeUnderConcurrentCalls(t *testing.T) { + sm := newTestSessionManager() + + const callers = 16 + done := make(chan struct{}, callers) + for i := 0; i < callers; i++ { + go func() { + sm.Stop() + done <- struct{}{} + }() + } + + for i := 0; i < callers; i++ { + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("concurrent Stop calls did not return") + } + } +} + +func TestSessionManagerStopWaitsForCleanupExit(t *testing.T) { + sm := newTestSessionManager() + + sm.Stop() + + select { + case <-sm.cleanupDone: + default: + t.Fatal("Stop returned before cleanup goroutine exited") + } +} diff --git a/packages/vm-agent/internal/server/server.go b/packages/vm-agent/internal/server/server.go index 23142e8a22..b6ce881530 100644 --- a/packages/vm-agent/internal/server/server.go +++ b/packages/vm-agent/internal/server/server.go @@ -108,6 +108,9 @@ type Server struct { callbackToken string httpClient *http.Client // shared HTTP client with timeout for control-plane callbacks done chan struct{} + stopOnce sync.Once + stopErrMu sync.Mutex + stopErr error publishJobsMu sync.Mutex publishJobs map[string]publishJobState buildPublishRunner func(context.Context, *preparedBuildPublish, publish.EventSink) (*publish.ReleaseResult, error) @@ -968,46 +971,57 @@ func (s *Server) StopAllWorkspacesAndSessions() { // Stop gracefully stops the server. func (s *Server) Stop(ctx context.Context) error { - // Signal background goroutines to stop. - close(s.done) + s.stopOnce.Do(func() { + // Signal background goroutines to stop. + close(s.done) - // Stop all port scanners - s.stopAllPortScanners() + // Stop all port scanners + s.stopAllPortScanners() - // Close JWT validator - s.jwtValidator.Close() + // Stop browser auth session cleanup. + s.sessionManager.Stop() - s.sessionHostMu.Lock() - for key, host := range s.sessionHosts { - if host != nil { - host.Stop() + // Close JWT validator + s.jwtValidator.Close() + + s.sessionHostMu.Lock() + for key, host := range s.sessionHosts { + if host != nil { + host.Stop() + } + delete(s.sessionHosts, key) } - delete(s.sessionHosts, key) - } - s.sessionHostMu.Unlock() + s.sessionHostMu.Unlock() - // Close all workspace PTY sessions. - s.workspaceMu.Lock() - for _, runtime := range s.workspaces { - runtime.PTY.CloseAllSessions() - } - s.workspaceMu.Unlock() + // Close all workspace PTY sessions. + s.workspaceMu.Lock() + for _, runtime := range s.workspaces { + runtime.PTY.CloseAllSessions() + } + s.workspaceMu.Unlock() - // Flush and stop error reporter - s.errorReporter.Shutdown() + // Flush and stop error reporter + s.errorReporter.Shutdown() - // Flush and stop all per-workspace message reporters - s.shutdownAllReporters() + // Flush and stop all per-workspace message reporters + s.shutdownAllReporters() - // Close persistence store - if s.store != nil { - if err := s.store.Close(); err != nil { - slog.Warn("Failed to close persistence store", "error", err) + // Close persistence store + if s.store != nil { + if err := s.store.Close(); err != nil { + slog.Warn("Failed to close persistence store", "error", err) + } } - } - // Shutdown HTTP server - return s.httpServer.Shutdown(ctx) + // Shutdown HTTP server + stopErr := s.httpServer.Shutdown(ctx) + s.stopErrMu.Lock() + s.stopErr = stopErr + s.stopErrMu.Unlock() + }) + s.stopErrMu.Lock() + defer s.stopErrMu.Unlock() + return s.stopErr } // setupRoutes configures the HTTP routes. diff --git a/packages/vm-agent/internal/server/shutdown_test.go b/packages/vm-agent/internal/server/shutdown_test.go new file mode 100644 index 0000000000..f0fdf3267f --- /dev/null +++ b/packages/vm-agent/internal/server/shutdown_test.go @@ -0,0 +1,116 @@ +package server + +import ( + "bytes" + "context" + "net/http" + "runtime/pprof" + "strings" + "sync" + "testing" + "time" + + "github.com/workspace/vm-agent/internal/acp" + "github.com/workspace/vm-agent/internal/auth" + "github.com/workspace/vm-agent/internal/config" + "github.com/workspace/vm-agent/internal/container" + "github.com/workspace/vm-agent/internal/errorreport" + "github.com/workspace/vm-agent/internal/messagereport" + "github.com/workspace/vm-agent/internal/ports" +) + +func newShutdownTestServer(t *testing.T) *Server { + t.Helper() + + sessionManager := auth.NewSessionManagerWithConfig(auth.SessionManagerConfig{ + CookieName: "vm_session", + Secure: false, + TTL: time.Hour, + CleanupInterval: time.Hour, + MaxSessions: 100, + }) + + errorReporter := errorreport.New("", "node-1", "", errorreport.Config{FlushInterval: time.Hour}) + errorReporter.Start() + + return &Server{ + config: &config.Config{}, + httpServer: &http.Server{}, + jwtValidator: &auth.JWTValidator{}, + sessionManager: sessionManager, + workspaces: make(map[string]*WorkspaceRuntime), + sessionHosts: make(map[string]*acp.SessionHost), + messageReporters: make(map[string]*messagereport.Reporter), + portScanners: make(map[string]*ports.Scanner), + portDiscoveries: make(map[string]*container.Discovery), + errorReporter: errorReporter, + done: make(chan struct{}), + } +} + +func authCleanupGoroutines(t *testing.T) int { + t.Helper() + + var buf bytes.Buffer + if err := pprof.Lookup("goroutine").WriteTo(&buf, 2); err != nil { + t.Fatalf("write goroutine profile: %v", err) + } + return strings.Count(buf.String(), "github.com/workspace/vm-agent/internal/auth.(*SessionManager).cleanup") +} + +func waitForAuthCleanupCount(t *testing.T, want int) { + t.Helper() + + deadline := time.After(2 * time.Second) + tick := time.NewTicker(10 * time.Millisecond) + defer tick.Stop() + + for { + if got := authCleanupGoroutines(t); got == want { + return + } + select { + case <-tick.C: + case <-deadline: + t.Fatalf("auth cleanup goroutine count = %d, want %d", authCleanupGoroutines(t), want) + } + } +} + +func TestServerStopIsIdempotentAndStopsAuthCleanup(t *testing.T) { + baseline := authCleanupGoroutines(t) + s := newShutdownTestServer(t) + waitForAuthCleanupCount(t, baseline+1) + + if err := s.Stop(context.Background()); err != nil { + t.Fatalf("first Stop returned error: %v", err) + } + if err := s.Stop(context.Background()); err != nil { + t.Fatalf("second Stop returned error: %v", err) + } + + waitForAuthCleanupCount(t, baseline) +} + +func TestServerStopIsSafeUnderConcurrentCalls(t *testing.T) { + s := newShutdownTestServer(t) + + const callers = 16 + var wg sync.WaitGroup + errs := make(chan error, callers) + wg.Add(callers) + for i := 0; i < callers; i++ { + go func() { + defer wg.Done() + errs <- s.Stop(context.Background()) + }() + } + wg.Wait() + close(errs) + + for err := range errs { + if err != nil { + t.Fatalf("concurrent Stop returned error: %v", err) + } + } +} diff --git a/tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md b/tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md index 09048f2f54..d51360c739 100644 --- a/tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md +++ b/tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md @@ -23,14 +23,14 @@ This task is a retry of the failed startup task `01KYQJ1P3FPQBDPCWFQ0SWPAYJ`; du ## Implementation checklist -- [ ] Make `auth.SessionManager.Stop()` idempotent and safe under repeated/concurrent calls without changing public API shape. -- [ ] Ensure `auth.SessionManager` cleanup goroutine exits when `Stop()` is called. -- [ ] Make `server.Server.Stop()` idempotent for repeated/concurrent calls without changing public API shape. -- [ ] Ensure `Server.Stop()` stops the owned auth session cleanup goroutine. -- [ ] Preserve current shutdown ordering for external behavior unless a narrow ordering change is required for complete cleanup. -- [ ] Add focused Go tests for repeated and concurrent `SessionManager.Stop()`. -- [ ] Add focused Go tests proving `Server.Stop()` calls auth session cleanup and repeated `Server.Stop()` does not panic. -- [ ] Run relevant Go tests, including `-race` if feasible. +- [x] Make `auth.SessionManager.Stop()` idempotent and safe under repeated/concurrent calls without changing public API shape. +- [x] Ensure `auth.SessionManager` cleanup goroutine exits when `Stop()` is called. +- [x] Make `server.Server.Stop()` idempotent for repeated/concurrent calls without changing public API shape. +- [x] Ensure `Server.Stop()` stops the owned auth session cleanup goroutine. +- [x] Preserve current shutdown ordering for external behavior unless a narrow ordering change is required for complete cleanup. +- [x] Add focused Go tests for repeated and concurrent `SessionManager.Stop()`. +- [x] Add focused Go tests proving `Server.Stop()` calls auth session cleanup and repeated `Server.Stop()` does not panic. +- [x] Run relevant Go tests, including `-race` if feasible. - [ ] Run local `go-specialist`, `test-engineer`, and task-completion validation reviews and address findings. - [ ] Open a PR against `main`, wait for CI, and do not merge. From 88ea02054573768f8b43d4d884f23c096c77a3bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:57:49 +0000 Subject: [PATCH 22/35] fix: make container max instances configurable --- .github/workflows/deploy-reusable.yml | 2 + scripts/deploy/sync-wrangler-config.ts | 65 +++++++++- .../quality/deploy-reusable-workflow.test.ts | 11 ++ scripts/quality/sync-wrangler-config.test.ts | 111 ++++++++++++++++++ ...29-configurable-container-max-instances.md | 16 +-- 5 files changed, 196 insertions(+), 9 deletions(-) diff --git a/.github/workflows/deploy-reusable.yml b/.github/workflows/deploy-reusable.yml index 7e6ad8bc54..bf5b6a2dc4 100644 --- a/.github/workflows/deploy-reusable.yml +++ b/.github/workflows/deploy-reusable.yml @@ -376,6 +376,8 @@ jobs: CF_CONTAINER_CREATE_WORKSPACE_TIMEOUT_MS: ${{ vars.CF_CONTAINER_CREATE_WORKSPACE_TIMEOUT_MS }} CF_CONTAINER_CLONE_FILTER: ${{ vars.CF_CONTAINER_CLONE_FILTER }} CF_CONTAINER_VM_AGENT_PORT: ${{ vars.CF_CONTAINER_VM_AGENT_PORT }} + SANDBOX_CONTAINER_MAX_INSTANCES: ${{ vars.SANDBOX_CONTAINER_MAX_INSTANCES }} + VM_AGENT_CONTAINER_MAX_INSTANCES: ${{ vars.VM_AGENT_CONTAINER_MAX_INSTANCES }} SANDBOX_ENABLED: ${{ vars.SANDBOX_ENABLED }} MAX_CONCURRENT_SETUP_SESSIONS: ${{ vars.MAX_CONCURRENT_SETUP_SESSIONS }} SETUP_SESSION_TTL_MS: ${{ vars.SETUP_SESSION_TTL_MS }} diff --git a/scripts/deploy/sync-wrangler-config.ts b/scripts/deploy/sync-wrangler-config.ts index 51a88a3bc6..952586654c 100644 --- a/scripts/deploy/sync-wrangler-config.ts +++ b/scripts/deploy/sync-wrangler-config.ts @@ -43,11 +43,30 @@ const TAIL_WORKER_WRANGLER_TOML_PATH = resolve( const DEPLOY_STATE_DIR = resolve(import.meta.dirname, '../../.wrangler'); const FIRST_DEPLOY_MARKER = resolve(DEPLOY_STATE_DIR, 'tail-worker-first-deploy'); const SETUP_TOKEN_BYTES = 24; +const DEFAULT_SANDBOX_CONTAINER_MAX_INSTANCES = 6; +const DEFAULT_VM_AGENT_CONTAINER_MAX_INSTANCES = 3; + +const CONTAINER_MAX_INSTANCE_CONFIG = { + SandboxDO: { + envVar: 'SANDBOX_CONTAINER_MAX_INSTANCES', + defaultValue: DEFAULT_SANDBOX_CONTAINER_MAX_INSTANCES, + }, + VmAgentContainer: { + envVar: 'VM_AGENT_CONTAINER_MAX_INSTANCES', + defaultValue: DEFAULT_VM_AGENT_CONTAINER_MAX_INSTANCES, + }, +} as const satisfies Record; const recordSchema = v.custom>( (value) => typeof value === 'object' && value !== null && !Array.isArray(value), 'Expected an object' ); +const positiveSafeIntegerSchema = v.pipe( + v.number(), + v.integer('must be an integer'), + v.minValue(1, 'must be greater than or equal to 1'), + v.safeInteger('must be a safe integer') +); function requireRecord(value: unknown, path: string): Record { const result = v.safeParse(recordSchema, value); @@ -84,6 +103,49 @@ function cloudflareWorkerVariablesUrl( return `https://dash.cloudflare.com/${accountId}/workers/services/view/${workerName}/${environment}/settings/variables`; } +function parseContainerMaxInstances(envVar: string, fallback: number): number { + const rawValue = process.env[envVar]; + if (!rawValue) { + return fallback; + } + + const trimmed = rawValue.trim(); + if (!/^\d+$/.test(trimmed)) { + throw new Error(`${envVar} must be a positive safe integer`); + } + + const parsed = Number(trimmed); + const result = v.safeParse(positiveSafeIntegerSchema, parsed); + if (!result.success) { + throw new Error(`${envVar} ${result.issues[0]?.message ?? 'must be a positive safe integer'}`); + } + return result.output; +} + +function getConfiguredContainerMaxInstances( + className: string, + currentValue: number | undefined +): number | undefined { + const config = + CONTAINER_MAX_INSTANCE_CONFIG[className as keyof typeof CONTAINER_MAX_INSTANCE_CONFIG]; + if (!config) { + return currentValue; + } + return parseContainerMaxInstances(config.envVar, config.defaultValue); +} + +function generateContainerBindings( + containers: ContainerBinding[] | undefined +): ContainerBinding[] | undefined { + return containers?.map((container) => ({ + ...container, + max_instances: getConfiguredContainerMaxInstances( + container.class_name, + container.max_instances + ), + })); +} + // ============================================================================ // Pulumi // ============================================================================ @@ -412,12 +474,13 @@ function getStaticApiWorkerBindings( analyticsEngineDatasets: AnalyticsEngineDatasetBinding[] | undefined, includeArtifactsBinding: boolean ): Partial { + const containers = generateContainerBindings(staticBindings.containers); return { ...(staticBindings.durable_objects ? { durable_objects: staticBindings.durable_objects } : {}), ...(staticBindings.ai ? { ai: staticBindings.ai } : {}), ...(analyticsEngineDatasets ? { analytics_engine_datasets: analyticsEngineDatasets } : {}), ...(staticBindings.migrations ? { migrations: staticBindings.migrations } : {}), - ...(staticBindings.containers ? { containers: staticBindings.containers } : {}), + ...(containers ? { containers } : {}), ...(includeArtifactsBinding ? { artifacts: staticBindings.artifacts } : {}), }; } diff --git a/scripts/quality/deploy-reusable-workflow.test.ts b/scripts/quality/deploy-reusable-workflow.test.ts index e696431d02..acd2787dd1 100644 --- a/scripts/quality/deploy-reusable-workflow.test.ts +++ b/scripts/quality/deploy-reusable-workflow.test.ts @@ -144,6 +144,17 @@ describe('deploy reusable workflow', () => { } }); + it('forwards Cloudflare container max-instance overrides into the wrangler config sync env', () => { + const sync = stepBlock('Sync Wrangler Config \\(API \\+ Tail Worker\\)'); + + expect(sync).toContain( + 'SANDBOX_CONTAINER_MAX_INSTANCES: ${{ vars.SANDBOX_CONTAINER_MAX_INSTANCES }}' + ); + expect(sync).toContain( + 'VM_AGENT_CONTAINER_MAX_INSTANCES: ${{ vars.VM_AGENT_CONTAINER_MAX_INSTANCES }}' + ); + }); + it('forwards the codex credential-setup tunables into the wrangler config sync env', () => { const sync = stepBlock('Sync Wrangler Config \\(API \\+ Tail Worker\\)'); diff --git a/scripts/quality/sync-wrangler-config.test.ts b/scripts/quality/sync-wrangler-config.test.ts index 27e13d43c4..074ef9df39 100644 --- a/scripts/quality/sync-wrangler-config.test.ts +++ b/scripts/quality/sync-wrangler-config.test.ts @@ -170,6 +170,117 @@ describe('sync wrangler config', () => { }); }); + it('generates Cloudflare container max_instances with unchanged safe defaults', () => { + vi.stubEnv('RESOURCE_PREFIX', 's123abc'); + + const containers = [ + { + class_name: 'SandboxDO', + image: './Dockerfile.sandbox', + instance_type: 'standard-1', + max_instances: 999, + }, + { + class_name: 'VmAgentContainer', + image: './Dockerfile.vm-agent-container', + instance_type: 'standard-1', + max_instances: 999, + }, + ]; + + const envConfig = generateApiWorkerEnv({ containers }, outputs, 'prod', false, false); + + expect(envConfig.containers).toEqual([ + { + class_name: 'SandboxDO', + image: './Dockerfile.sandbox', + instance_type: 'standard-1', + max_instances: 6, + }, + { + class_name: 'VmAgentContainer', + image: './Dockerfile.vm-agent-container', + instance_type: 'standard-1', + max_instances: 3, + }, + ]); + }); + + it('respects deployment overrides for Cloudflare container max_instances', () => { + vi.stubEnv('RESOURCE_PREFIX', 's123abc'); + vi.stubEnv('SANDBOX_CONTAINER_MAX_INSTANCES', '8'); + vi.stubEnv('VM_AGENT_CONTAINER_MAX_INSTANCES', '5'); + + const containers = [ + { + class_name: 'SandboxDO', + image: './Dockerfile.sandbox', + instance_type: 'standard-1', + max_instances: 6, + }, + { + class_name: 'VmAgentContainer', + image: './Dockerfile.vm-agent-container', + instance_type: 'standard-1', + max_instances: 3, + }, + { + class_name: 'OtherContainer', + image: './Dockerfile.other', + instance_type: 'standard-1', + max_instances: 2, + }, + ]; + + const envConfig = generateApiWorkerEnv({ containers }, outputs, 'prod', false, false); + + expect(envConfig.containers).toEqual([ + { + class_name: 'SandboxDO', + image: './Dockerfile.sandbox', + instance_type: 'standard-1', + max_instances: 8, + }, + { + class_name: 'VmAgentContainer', + image: './Dockerfile.vm-agent-container', + instance_type: 'standard-1', + max_instances: 5, + }, + { + class_name: 'OtherContainer', + image: './Dockerfile.other', + instance_type: 'standard-1', + max_instances: 2, + }, + ]); + }); + + it('fails closed for invalid Cloudflare container max_instances overrides', () => { + vi.stubEnv('RESOURCE_PREFIX', 's123abc'); + vi.stubEnv('SANDBOX_CONTAINER_MAX_INSTANCES', '0'); + + const topLevel: WranglerToml = { + containers: [ + { + class_name: 'SandboxDO', + image: './Dockerfile.sandbox', + instance_type: 'standard-1', + max_instances: 6, + }, + ], + }; + + expect(() => generateApiWorkerEnv(topLevel, outputs, 'prod', false, false)).toThrow( + 'SANDBOX_CONTAINER_MAX_INSTANCES must be greater than or equal to 1' + ); + + vi.stubEnv('SANDBOX_CONTAINER_MAX_INSTANCES', '1.5'); + expect(() => generateApiWorkerEnv(topLevel, outputs, 'prod', false, false)).toThrow( + 'SANDBOX_CONTAINER_MAX_INSTANCES must be a positive safe integer' + ); + }); + it('omits Artifacts binding and disables runtime flag when Artifacts is not enabled', () => { vi.stubEnv('RESOURCE_PREFIX', 's123abc'); diff --git a/tasks/active/2026-07-29-configurable-container-max-instances.md b/tasks/active/2026-07-29-configurable-container-max-instances.md index 9e622d16c8..9c91a81ab0 100644 --- a/tasks/active/2026-07-29-configurable-container-max-instances.md +++ b/tasks/active/2026-07-29-configurable-container-max-instances.md @@ -17,14 +17,14 @@ Cloudflare container `max_instances` is checked into `apps/api/wrangler.toml` as ## Implementation checklist -- [ ] Add centralized container max-instance config in the Wrangler sync script with defaults exactly `SandboxDO=6` and `VmAgentContainer=3`. -- [ ] Add generic deployment environment variable names for overriding those limits. -- [ ] Validate overrides as positive safe integers before generating Wrangler config. -- [ ] Generate/sync container blocks from centralized config instead of copying static `max_instances` through unchanged. -- [ ] Forward the new optional variables from `.github/workflows/deploy-reusable.yml`. -- [ ] Add quality tests proving defaults remain `6` and `3`. -- [ ] Add quality tests proving overrides are respected. -- [ ] Add quality tests proving invalid overrides fail closed. +- [x] Add centralized container max-instance config in the Wrangler sync script with defaults exactly `SandboxDO=6` and `VmAgentContainer=3`. +- [x] Add generic deployment environment variable names for overriding those limits. +- [x] Validate overrides as positive safe integers before generating Wrangler config. +- [x] Generate/sync container blocks from centralized config instead of copying static `max_instances` through unchanged. +- [x] Forward the new optional variables from `.github/workflows/deploy-reusable.yml`. +- [x] Add quality tests proving defaults remain `6` and `3`. +- [x] Add quality tests proving overrides are respected. +- [x] Add quality tests proving invalid overrides fail closed. - [ ] Run local Cloudflare/config/test specialist reviews and address findings. - [ ] Open a PR against `main`, wait for CI, and do not merge. From 9a11ab917cdd04d12ed5cff007107e43c0c5cc95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:52:00 +0000 Subject: [PATCH 23/35] fix: isolate malformed project data message rows --- .../durable-objects/project-data/messages.ts | 35 ++++++++- apps/api/src/services/bootstrap.ts | 4 +- .../project-data-messages.test.ts | 71 ++++++++++++++++++- .../api/tests/unit/services/bootstrap.test.ts | 4 +- ...6-07-29-project-data-list-row-isolation.md | 12 ++-- 5 files changed, 112 insertions(+), 14 deletions(-) rename tasks/{backlog => active}/2026-07-29-project-data-list-row-isolation.md (84%) diff --git a/apps/api/src/durable-objects/project-data/messages.ts b/apps/api/src/durable-objects/project-data/messages.ts index de92f08e78..96ce0e8af7 100644 --- a/apps/api/src/durable-objects/project-data/messages.ts +++ b/apps/api/src/durable-objects/project-data/messages.ts @@ -406,10 +406,39 @@ export function getMessages( const trimmedRows = candidateRows.slice(0, safeCount); const orderedRows = order === 'desc' ? trimmedRows.reverse() : trimmedRows; + const messages: Record[] = []; + let skipped = 0; + + for (const row of orderedRows) { + try { + messages.push( + compact ? parseChatMessageRowCompact(row, compactOptions) : parseChatMessageRow(row) + ); + } catch (e) { + skipped++; + log.warn('messages.list_row_skipped', { + rowId: typeof row.id === 'string' ? row.id : null, + rowSessionId: typeof row.session_id === 'string' ? row.session_id : null, + requestedSessionId: sessionId, + compact, + error: String(e), + }); + } + } + + if (skipped > 0) { + log.warn('messages.list_degraded', { + sessionId, + requestedLimit: limit, + fetched: rows.length, + returned: messages.length, + skipped, + compact, + }); + } + return { - messages: orderedRows.map((row) => - compact ? parseChatMessageRowCompact(row, compactOptions) : parseChatMessageRow(row) - ), + messages, hasMore, }; } diff --git a/apps/api/src/services/bootstrap.ts b/apps/api/src/services/bootstrap.ts index 835173b101..cc14bb7be5 100644 --- a/apps/api/src/services/bootstrap.ts +++ b/apps/api/src/services/bootstrap.ts @@ -2,7 +2,7 @@ * Bootstrap Token Service * * Manages one-time bootstrap tokens for secure credential delivery to VMs. - * Tokens are stored in KV with a 15-minute TTL and are deleted after single use. + * Tokens are stored in KV with a configurable TTL and are deleted after single use. */ import type { BootstrapTokenData } from '@simple-agent-manager/shared'; @@ -15,7 +15,7 @@ import { decrypt, encrypt } from './encryption'; const BOOTSTRAP_PREFIX = 'bootstrap:'; const inFlightRedemptions = new Map>(); -/** Default bootstrap token TTL in seconds (15 minutes) */ +/** Default bootstrap token TTL in seconds. */ const DEFAULT_BOOTSTRAP_TTL = 900; const LEGACY_CLOCK_SKEW_SECONDS = 60; diff --git a/apps/api/tests/unit/durable-objects/project-data-messages.test.ts b/apps/api/tests/unit/durable-objects/project-data-messages.test.ts index 3137d6a88a..b29697991c 100644 --- a/apps/api/tests/unit/durable-objects/project-data-messages.test.ts +++ b/apps/api/tests/unit/durable-objects/project-data-messages.test.ts @@ -1,6 +1,7 @@ -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { getMessages } from '../../../src/durable-objects/project-data/messages'; +import { log } from '../../../src/lib/logger'; type QueryRow = Record; @@ -26,6 +27,10 @@ function makeSql(rows: QueryRow[]) { } describe('ProjectData messages getMessages', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + it('keeps newest-page default behavior ordered chronologically for rendering', () => { const newest = makeRow({ id: 'newest', content: 'Newest', created_at: 3000, sequence: 3 }); const older = makeRow({ id: 'older', content: 'Older', created_at: 2000, sequence: 2 }); @@ -38,6 +43,70 @@ describe('ProjectData messages getMessages', () => { expect(result.hasMore).toBe(false); }); + it('skips a malformed message row among valid rows instead of throwing', () => { + const warn = vi.spyOn(log, 'warn').mockImplementation(() => {}); + const newest = makeRow({ id: 'newest', content: 'Newest', created_at: 3000, sequence: 3 }); + const bad = makeRow({ id: 'bad', content: null, created_at: 2000, sequence: 2 }); + const oldest = makeRow({ id: 'oldest', content: 'Oldest', created_at: 1000, sequence: 1 }); + const sql = makeSql([newest, bad, oldest]); + + expect(() => getMessages(sql, 'session-1', 3)).not.toThrow(); + + const result = getMessages(sql, 'session-1', 3); + expect(result.messages.map((message) => message.id)).toEqual(['oldest', 'newest']); + expect(result.hasMore).toBe(false); + expect(warn).toHaveBeenCalledWith( + 'messages.list_row_skipped', + expect.objectContaining({ + rowId: 'bad', + rowSessionId: 'session-1', + requestedSessionId: 'session-1', + compact: false, + error: expect.stringContaining('content'), + }) + ); + expect(warn).toHaveBeenCalledWith( + 'messages.list_degraded', + expect.objectContaining({ returned: 2, skipped: 1 }) + ); + }); + + it('skips malformed compact message rows without failing the compact list', () => { + const warn = vi.spyOn(log, 'warn').mockImplementation(() => {}); + const good = makeRow({ id: 'good', content: 'Good compact content' }); + const bad = makeRow({ id: 'bad-compact', content: null }); + const sql = makeSql([good, bad]); + + const result = getMessages(sql, 'session-1', 2, null, undefined, true); + + expect(result.messages.map((message) => message.id)).toEqual(['good']); + expect(warn).toHaveBeenCalledWith( + 'messages.list_row_skipped', + expect.objectContaining({ + rowId: 'bad-compact', + compact: true, + error: expect.stringContaining('content'), + }) + ); + }); + + it('returns an empty non-throwing list when every message row is malformed', () => { + const warn = vi.spyOn(log, 'warn').mockImplementation(() => {}); + const sql = makeSql([ + makeRow({ id: 'bad-1', content: null }), + makeRow({ id: 'bad-2', role: null }), + ]); + + const result = getMessages(sql, 'session-1', 2); + + expect(result.messages).toEqual([]); + expect(result.hasMore).toBe(false); + expect(warn).toHaveBeenCalledWith( + 'messages.list_degraded', + expect.objectContaining({ returned: 0, skipped: 2 }) + ); + }); + it('supports oldest-first lookups for the initial user prompt', () => { const initialPrompt = makeRow({ id: 'initial', content: 'Initial prompt', created_at: 1000, sequence: 1 }); const followUp = makeRow({ id: 'follow-up', content: 'Follow-up prompt', created_at: 3000, sequence: 3 }); diff --git a/apps/api/tests/unit/services/bootstrap.test.ts b/apps/api/tests/unit/services/bootstrap.test.ts index 6e8a014bff..b7a6bb3a7d 100644 --- a/apps/api/tests/unit/services/bootstrap.test.ts +++ b/apps/api/tests/unit/services/bootstrap.test.ts @@ -45,7 +45,7 @@ describe('Bootstrap Service', () => { }); describe('storeBootstrapToken', () => { - it('should store token data in KV with 15-minute TTL', async () => { + it('should store token data in KV with the default TTL', async () => { const { storeBootstrapToken } = await import( '../../../src/services/bootstrap' ); @@ -167,7 +167,7 @@ describe('Bootstrap Service', () => { mockEnv ); - // Verify TTL is set to 900 seconds (15 minutes) + // Verify the default TTL is used when no override is configured expect(mockKV.put).toHaveBeenCalledWith( expect.any(String), expect.any(String), diff --git a/tasks/backlog/2026-07-29-project-data-list-row-isolation.md b/tasks/active/2026-07-29-project-data-list-row-isolation.md similarity index 84% rename from tasks/backlog/2026-07-29-project-data-list-row-isolation.md rename to tasks/active/2026-07-29-project-data-list-row-isolation.md index cbcd023b3d..65ea36b521 100644 --- a/tasks/backlog/2026-07-29-project-data-list-row-isolation.md +++ b/tasks/active/2026-07-29-project-data-list-row-isolation.md @@ -16,12 +16,12 @@ Also fix the stale bootstrap TTL comment/test wording so it remains correct when ## Checklist -- [ ] Add row-level parse isolation to `getMessages()` for normal and compact message rows. -- [ ] Warn-log skipped message rows with context, best-effort row id/session id, compact mode, and parser error. -- [ ] Preserve the existing `{ messages, hasMore }` response contract and ordering behavior. -- [ ] Add a good/bad/good regression test for malformed message rows. -- [ ] Add an all-bad regression test returning an empty non-throwing list. -- [ ] Update stale bootstrap TTL comment/test wording without changing runtime behavior. +- [x] Add row-level parse isolation to `getMessages()` for normal and compact message rows. +- [x] Warn-log skipped message rows with context, best-effort row id/session id, compact mode, and parser error. +- [x] Preserve the existing `{ messages, hasMore }` response contract and ordering behavior. +- [x] Add a good/bad/good regression test for malformed message rows. +- [x] Add an all-bad regression test returning an empty non-throwing list. +- [x] Update stale bootstrap TTL comment/test wording without changing runtime behavior. - [ ] Run targeted tests and broader validation. - [ ] Run local reviewer/subagent checks for tests and code review. - [ ] Open PR, wait for CI, and do not merge. From c9b69d45b7a871f89e9ef09fe5166d9a736afe18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 18:56:41 +0000 Subject: [PATCH 24/35] chore: save agent work Auto-committed by SAM on agent completion. --- .codex/config.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/.codex/config.toml b/.codex/config.toml index 2eba2390c5..545a202d79 100644 --- a/.codex/config.toml +++ b/.codex/config.toml @@ -18,7 +18,6 @@ timeout = 30000 # Added by SAM vm-agent for Codex ACP sessions. sandbox_mode = "danger-full-access" approval_policy = "never" -model_reasoning_effort = "high" [mcp_servers.sam-mcp] url = "https://api.simple-agent-manager.org/mcp" bearer_token_env_var = "SAM_MCP_TOKEN" From 60a2ac1479a013313e482a7fb47f5f868071087b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:12:59 +0000 Subject: [PATCH 25/35] fix: satisfy sonar for wrangler inventory check --- scripts/quality/check-wrangler-bindings.ts | 171 +++++++++++---------- 1 file changed, 86 insertions(+), 85 deletions(-) diff --git a/scripts/quality/check-wrangler-bindings.ts b/scripts/quality/check-wrangler-bindings.ts index e46010fbdd..fb96b2774d 100644 --- a/scripts/quality/check-wrangler-bindings.ts +++ b/scripts/quality/check-wrangler-bindings.ts @@ -26,6 +26,8 @@ const TAIL_WORKER_WRANGLER_PATH = resolve( '../../apps/tail-worker/wrangler.toml' ); const CONFIGURE_SECRETS_PATH = resolve(import.meta.dirname, '../deploy/configure-secrets.sh'); +const SECRET_COMMENT_PATTERN = /^# - `?([A-Z0-9_]+)`?/; +const sortAlphabetically = (left: string, right: string): number => left.localeCompare(right); interface Binding { name?: string; @@ -67,7 +69,7 @@ function extractConfiguredWorkerSecrets(scriptContent: string): string[] { new Set( Array.from(scriptContent.matchAll(/set_worker_secret\s+"([A-Z0-9_]+)"/g), (match) => match[1]) ) - ).sort(); + ).sort(sortAlphabetically); } function extractWranglerCommentedSecrets(wranglerContent: string): string[] { @@ -90,13 +92,13 @@ function extractWranglerCommentedSecrets(wranglerContent: string): string[] { if (!line.startsWith('#')) { break; } - const match = line.match(/^# - `?([A-Z0-9_]+)`?/); + const match = SECRET_COMMENT_PATTERN.exec(line); if (match) { secrets.push(match[1]); } } - return Array.from(new Set(secrets)).sort(); + return Array.from(new Set(secrets)).sort(sortAlphabetically); } function diffSecrets(expected: string[], actual: string[]): { missing: string[]; extra: string[] } { @@ -108,104 +110,81 @@ function diffSecrets(expected: string[], actual: string[]): { missing: string[]; }; } -function main(): void { - const errors: string[] = []; - - // ======================================== - // Check 1: No env sections committed - // ======================================== - - const apiContent = readFileSync(API_WRANGLER_PATH, 'utf-8'); - const apiConfig = TOML.parse(apiContent) as unknown as WranglerConfig; - const configureSecretsContent = readFileSync(CONFIGURE_SECRETS_PATH, 'utf-8'); - - if (apiConfig.env && Object.keys(apiConfig.env).length > 0) { - const envNames = Object.keys(apiConfig.env).join(', '); - errors.push( - `apps/api/wrangler.toml contains [env.*] sections (${envNames}). ` + - `These are generated at deploy time by sync-wrangler-config.ts. ` + - `Remove them from the checked-in file.` - ); +function checkNoCommittedEnvSections(errors: string[], config: WranglerConfig, path: string): void { + if (!config.env || Object.keys(config.env).length === 0) { + return; } - const tailContent = readFileSync(TAIL_WORKER_WRANGLER_PATH, 'utf-8'); - const tailConfig = TOML.parse(tailContent) as unknown as WranglerConfig; + const envNames = Object.keys(config.env).join(', '); + errors.push( + `${path} contains [env.*] sections (${envNames}). These are generated at deploy time. Remove them from the checked-in file.` + ); +} - if (tailConfig.env && Object.keys(tailConfig.env).length > 0) { - const envNames = Object.keys(tailConfig.env).join(', '); - errors.push( - `apps/tail-worker/wrangler.toml contains [env.*] sections (${envNames}). ` + - `These are generated at deploy time. Remove them from the checked-in file.` - ); +function checkRequiredApiBindings(errors: string[], apiConfig: WranglerConfig): void { + const requiredBindingChecks: Array<{ isPresent: boolean; message: string }> = [ + { + isPresent: Boolean(apiConfig.durable_objects?.bindings?.length), + message: + 'apps/api/wrangler.toml: top-level missing durable_objects.bindings (sync script copies these to env sections)', + }, + { + isPresent: Boolean(apiConfig.ai?.binding), + message: + 'apps/api/wrangler.toml: top-level missing [ai] binding (sync script copies this to env sections)', + }, + { + isPresent: Boolean(apiConfig.d1_databases?.length), + message: 'apps/api/wrangler.toml: top-level missing d1_databases', + }, + { + isPresent: Boolean(apiConfig.kv_namespaces?.length), + message: 'apps/api/wrangler.toml: top-level missing kv_namespaces', + }, + { + isPresent: Boolean(apiConfig.r2_buckets?.length), + message: 'apps/api/wrangler.toml: top-level missing r2_buckets', + }, + { + isPresent: Boolean(apiConfig.migrations?.length), + message: + 'apps/api/wrangler.toml: top-level missing [[migrations]] (sync script copies these to env sections)', + }, + ]; + + for (const check of requiredBindingChecks) { + if (!check.isPresent) { + errors.push(check.message); + } } +} - // ======================================== - // Check 2: Top-level has required bindings - // ======================================== - - if (!apiConfig.durable_objects?.bindings?.length) { +function checkSecretInventory( + errors: string[], + configuredSecrets: string[], + commentedSecrets: string[] +): void { + if (commentedSecrets.length === 0) { errors.push( - 'apps/api/wrangler.toml: top-level missing durable_objects.bindings (sync script copies these to env sections)' + 'apps/api/wrangler.toml: missing "# Secrets (set via wrangler secret put):" inventory comment' ); + return; } - if (!apiConfig.ai?.binding) { + const { missing, extra } = diffSecrets(configuredSecrets, commentedSecrets); + if (missing.length > 0) { errors.push( - 'apps/api/wrangler.toml: top-level missing [ai] binding (sync script copies this to env sections)' + `apps/api/wrangler.toml secret inventory is missing configured secrets from configure-secrets.sh: ${missing.join(', ')}` ); } - - if (!apiConfig.d1_databases?.length) { - errors.push('apps/api/wrangler.toml: top-level missing d1_databases'); - } - - if (!apiConfig.kv_namespaces?.length) { - errors.push('apps/api/wrangler.toml: top-level missing kv_namespaces'); - } - - if (!apiConfig.r2_buckets?.length) { - errors.push('apps/api/wrangler.toml: top-level missing r2_buckets'); - } - - if (!apiConfig.migrations?.length) { + if (extra.length > 0) { errors.push( - 'apps/api/wrangler.toml: top-level missing [[migrations]] (sync script copies these to env sections)' + `apps/api/wrangler.toml secret inventory lists secrets not configured by configure-secrets.sh: ${extra.join(', ')}` ); } +} - // ======================================== - // Check 3: Worker secret inventory comment matches configure-secrets.sh - // ======================================== - - const configuredSecrets = extractConfiguredWorkerSecrets(configureSecretsContent); - const commentedSecrets = extractWranglerCommentedSecrets(apiContent); - - if (commentedSecrets.length === 0) { - errors.push( - 'apps/api/wrangler.toml: missing "# Secrets (set via wrangler secret put):" inventory comment' - ); - } else { - const { missing, extra } = diffSecrets(configuredSecrets, commentedSecrets); - if (missing.length > 0) { - errors.push( - `apps/api/wrangler.toml secret inventory is missing configured secrets from configure-secrets.sh: ${missing.join(', ')}` - ); - } - if (extra.length > 0) { - errors.push( - `apps/api/wrangler.toml secret inventory lists secrets not configured by configure-secrets.sh: ${extra.join(', ')}` - ); - } - } - - // ======================================== - // Result - // ======================================== - - if (errors.length > 0) { - fail(errors); - } - +function logSuccess(apiConfig: WranglerConfig, configuredSecrets: string[]): void { const doCount = apiConfig.durable_objects?.bindings?.length ?? 0; const d1Count = apiConfig.d1_databases?.length ?? 0; const kvCount = apiConfig.kv_namespaces?.length ?? 0; @@ -222,4 +201,26 @@ function main(): void { ); } +function main(): void { + const errors: string[] = []; + const apiContent = readFileSync(API_WRANGLER_PATH, 'utf-8'); + const apiConfig = TOML.parse(apiContent) as unknown as WranglerConfig; + const configureSecretsContent = readFileSync(CONFIGURE_SECRETS_PATH, 'utf-8'); + const tailContent = readFileSync(TAIL_WORKER_WRANGLER_PATH, 'utf-8'); + const tailConfig = TOML.parse(tailContent) as unknown as WranglerConfig; + const configuredSecrets = extractConfiguredWorkerSecrets(configureSecretsContent); + const commentedSecrets = extractWranglerCommentedSecrets(apiContent); + + checkNoCommittedEnvSections(errors, apiConfig, 'apps/api/wrangler.toml'); + checkNoCommittedEnvSections(errors, tailConfig, 'apps/tail-worker/wrangler.toml'); + checkRequiredApiBindings(errors, apiConfig); + checkSecretInventory(errors, configuredSecrets, commentedSecrets); + + if (errors.length > 0) { + fail(errors); + } + + logSuccess(apiConfig, configuredSecrets); +} + main(); From 1d5d06187ef466210c7fa24db4f99c038cf0caf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:03:47 +0000 Subject: [PATCH 26/35] task: archive vm-agent shutdown idempotency --- ...026-07-29-vm-agent-shutdown-idempotency.md | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) rename tasks/{active => archive}/2026-07-29-vm-agent-shutdown-idempotency.md (73%) diff --git a/tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md b/tasks/archive/2026-07-29-vm-agent-shutdown-idempotency.md similarity index 73% rename from tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md rename to tasks/archive/2026-07-29-vm-agent-shutdown-idempotency.md index d51360c739..b3e1053dd2 100644 --- a/tasks/active/2026-07-29-vm-agent-shutdown-idempotency.md +++ b/tasks/archive/2026-07-29-vm-agent-shutdown-idempotency.md @@ -31,7 +31,7 @@ This task is a retry of the failed startup task `01KYQJ1P3FPQBDPCWFQ0SWPAYJ`; du - [x] Add focused Go tests for repeated and concurrent `SessionManager.Stop()`. - [x] Add focused Go tests proving `Server.Stop()` calls auth session cleanup and repeated `Server.Stop()` does not panic. - [x] Run relevant Go tests, including `-race` if feasible. -- [ ] Run local `go-specialist`, `test-engineer`, and task-completion validation reviews and address findings. +- [x] Run local `go-specialist`, `test-engineer`, and task-completion validation reviews and address findings. - [ ] Open a PR against `main`, wait for CI, and do not merge. ## Acceptance criteria @@ -42,3 +42,22 @@ This task is a retry of the failed startup task `01KYQJ1P3FPQBDPCWFQ0SWPAYJ`; du - No external/public API changes are introduced. - Strong Go regression tests cover the shutdown idempotency contract and pass locally. - PR includes specialist review evidence, tests run, CI status, and a no-breaking-change rationale. + + +## Validation evidence + +- `go test ./internal/auth ./internal/server` — passed. +- `go test -race ./internal/auth ./internal/server` — passed. +- `go test ./...` from `packages/vm-agent` — passed. + +## Specialist review evidence + +| Reviewer | Status | Outcome | +| --- | --- | --- | +| task-completion-validator | PASS | Research findings, checked checklist items, and acceptance criteria are represented in the diff/tests; no UI or multi-resource paths apply. | +| go-specialist | PASS | Concurrency/resource lifecycle is bounded by `sync.Once`; no mutex is held during I/O; auth cleanup and server background shutdown are explicit. | +| test-engineer | PASS | Regression tests cover repeated and concurrent Stop calls, cleanup goroutine exit, and race-targeted execution. | + +## No-breaking-change rationale + +The change adds only unexported synchronization fields and internal shutdown coordination. Existing constructors, methods, HTTP routes, config fields, and response shapes are unchanged. First-call shutdown behavior is preserved while repeated calls become safe no-ops that return the first shutdown result. From 2f79afcf6b0b1ad9dd9001a120a02c7bc24ae8f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:07:22 +0000 Subject: [PATCH 27/35] task: archive container max instances config --- .../2026-07-29-configurable-container-max-instances.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename tasks/{active => archive}/2026-07-29-configurable-container-max-instances.md (96%) diff --git a/tasks/active/2026-07-29-configurable-container-max-instances.md b/tasks/archive/2026-07-29-configurable-container-max-instances.md similarity index 96% rename from tasks/active/2026-07-29-configurable-container-max-instances.md rename to tasks/archive/2026-07-29-configurable-container-max-instances.md index 9c91a81ab0..e29beb2b55 100644 --- a/tasks/active/2026-07-29-configurable-container-max-instances.md +++ b/tasks/archive/2026-07-29-configurable-container-max-instances.md @@ -25,8 +25,8 @@ Cloudflare container `max_instances` is checked into `apps/api/wrangler.toml` as - [x] Add quality tests proving defaults remain `6` and `3`. - [x] Add quality tests proving overrides are respected. - [x] Add quality tests proving invalid overrides fail closed. -- [ ] Run local Cloudflare/config/test specialist reviews and address findings. -- [ ] Open a PR against `main`, wait for CI, and do not merge. +- [x] Run local Cloudflare/config/test specialist reviews and address findings. +- [x] Open a PR against `main`, wait for CI, and do not merge. ## Acceptance criteria From 6f2c6078c3a788cf0df8b5b9436cdf40399f3799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:30:31 +0000 Subject: [PATCH 28/35] task: archive project data row isolation --- .../2026-07-29-project-data-list-row-isolation.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename tasks/{active => archive}/2026-07-29-project-data-list-row-isolation.md (93%) diff --git a/tasks/active/2026-07-29-project-data-list-row-isolation.md b/tasks/archive/2026-07-29-project-data-list-row-isolation.md similarity index 93% rename from tasks/active/2026-07-29-project-data-list-row-isolation.md rename to tasks/archive/2026-07-29-project-data-list-row-isolation.md index 65ea36b521..252aff36a2 100644 --- a/tasks/active/2026-07-29-project-data-list-row-isolation.md +++ b/tasks/archive/2026-07-29-project-data-list-row-isolation.md @@ -22,9 +22,9 @@ Also fix the stale bootstrap TTL comment/test wording so it remains correct when - [x] Add a good/bad/good regression test for malformed message rows. - [x] Add an all-bad regression test returning an empty non-throwing list. - [x] Update stale bootstrap TTL comment/test wording without changing runtime behavior. -- [ ] Run targeted tests and broader validation. -- [ ] Run local reviewer/subagent checks for tests and code review. -- [ ] Open PR, wait for CI, and do not merge. +- [x] Run targeted tests and broader validation. +- [x] Run local reviewer/subagent checks for tests and code review. +- [x] Open PR, wait for CI, and do not merge. ## Acceptance criteria From 787b2db0beb24f13ae45392b39580b03bd33a07c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 19:33:10 +0000 Subject: [PATCH 29/35] chore: retrigger CI after PR body update From 3f06b8d685515058e9c5f28d9c291972f8ca83e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 20:38:07 +0000 Subject: [PATCH 30/35] task: update integration validation tracking --- .../2026-07-29-strict-cto-remediation-mega-pr.md | 12 ++++++------ .../2026-07-29-workersdev-cron-fail-closed.md | 0 2 files changed, 6 insertions(+), 6 deletions(-) rename tasks/{active => archive}/2026-07-29-workersdev-cron-fail-closed.md (100%) diff --git a/tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md b/tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md index 648632fc04..b6bdad4a2e 100644 --- a/tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md +++ b/tasks/active/2026-07-29-strict-cto-remediation-mega-pr.md @@ -24,12 +24,12 @@ Eight remediation PRs have been completed independently and need to be integrate ## Checklist -- [ ] Create integration branch from current `origin/main`. -- [ ] Merge/cherry-pick PRs #1689 through #1696. -- [ ] Resolve conflicts without dropping tests or docs from any remediation. -- [ ] Run targeted tests for all affected areas. -- [ ] Run full feasible local validation: lint, typecheck, tests, build. -- [ ] Run local specialist reviews for correctness, security, Cloudflare/env consistency, Go quality, task completion, and test quality. +- [x] Create integration branch from current `origin/main`. +- [x] Merge/cherry-pick PRs #1689 through #1696. +- [x] Resolve conflicts without dropping tests or docs from any remediation. +- [x] Run targeted tests for all affected areas. +- [x] Run full feasible local validation: lint, typecheck, tests, build. +- [x] Run local specialist reviews for correctness, security, Cloudflare/env consistency, Go quality, task completion, and test quality. - [ ] Open mega PR with specialist review evidence. - [ ] Wait for CI to be completely green. - [ ] Check staging state before deploy and avoid clobbering active validation. diff --git a/tasks/active/2026-07-29-workersdev-cron-fail-closed.md b/tasks/archive/2026-07-29-workersdev-cron-fail-closed.md similarity index 100% rename from tasks/active/2026-07-29-workersdev-cron-fail-closed.md rename to tasks/archive/2026-07-29-workersdev-cron-fail-closed.md From 102125f18918446327be5cc5dbd3ca3674bd984d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 29 Jul 2026 20:42:11 +0000 Subject: [PATCH 31/35] fix: avoid YAML anchors in deploy workflow --- .github/workflows/deploy-reusable.yml | 36 +++++++++++++++++-- .../quality/deploy-reusable-workflow.test.ts | 15 +++++--- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/.github/workflows/deploy-reusable.yml b/.github/workflows/deploy-reusable.yml index fb0b1b2607..2a22bc8980 100644 --- a/.github/workflows/deploy-reusable.yml +++ b/.github/workflows/deploy-reusable.yml @@ -347,7 +347,7 @@ jobs: - name: Sync Wrangler Config (API + Tail Worker) if: ${{ inputs.dry_run != true }} run: pnpm tsx scripts/deploy/sync-wrangler-config.ts - env: &wrangler_sync_env + env: PULUMI_STACK: ${{ steps.pulumi-select.outputs.stack_name }} AWS_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.R2_SECRET_ACCESS_KEY }} @@ -683,7 +683,39 @@ jobs: rm -f .wrangler/tail-worker-first-deploy pnpm tsx scripts/deploy/sync-wrangler-config.ts env: - <<: *wrangler_sync_env + PULUMI_STACK: ${{ steps.pulumi-select.outputs.stack_name }} + AWS_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.R2_SECRET_ACCESS_KEY }} + PULUMI_CONFIG_PASSPHRASE: ${{ secrets.PULUMI_CONFIG_PASSPHRASE }} + CF_API_TOKEN: ${{ secrets.CF_API_TOKEN }} + CLOUDFLARE_API_TOKEN: ${{ secrets.CF_API_TOKEN }} + BASE_DOMAIN: ${{ vars.BASE_DOMAIN }} + RESOURCE_PREFIX: ${{ steps.prefix.outputs.value }} + REQUIRE_APPROVAL: ${{ vars.REQUIRE_APPROVAL }} + SETUP_FORCE: ${{ vars.SETUP_FORCE }} + HETZNER_BASE_IMAGE: ${{ vars.HETZNER_BASE_IMAGE }} + ARTIFACTS_BINDING_ENABLED: ${{ vars.ARTIFACTS_BINDING_ENABLED }} + CF_CONTAINER_ENABLED: ${{ vars.CF_CONTAINER_ENABLED }} + CF_CONTAINER_SLEEP_AFTER: ${{ vars.CF_CONTAINER_SLEEP_AFTER }} + CF_CONTAINER_ACTIVE_WORK_MAX_MS: ${{ vars.CF_CONTAINER_ACTIVE_WORK_MAX_MS }} + CF_CONTAINER_KEEPALIVE_RENEW_INTERVAL_MS: ${{ vars.CF_CONTAINER_KEEPALIVE_RENEW_INTERVAL_MS }} + CF_CONTAINER_PORT_READY_TIMEOUT_MS: ${{ vars.CF_CONTAINER_PORT_READY_TIMEOUT_MS }} + CF_CONTAINER_WAKE_TIMEOUT_MS: ${{ vars.CF_CONTAINER_WAKE_TIMEOUT_MS }} + CF_CONTAINER_RECOVERY_MAX_ATTEMPTS: ${{ vars.CF_CONTAINER_RECOVERY_MAX_ATTEMPTS }} + CF_CONTAINER_CREATE_WORKSPACE_TIMEOUT_MS: ${{ vars.CF_CONTAINER_CREATE_WORKSPACE_TIMEOUT_MS }} + CF_CONTAINER_CLONE_FILTER: ${{ vars.CF_CONTAINER_CLONE_FILTER }} + CF_CONTAINER_VM_AGENT_PORT: ${{ vars.CF_CONTAINER_VM_AGENT_PORT }} + SANDBOX_CONTAINER_MAX_INSTANCES: ${{ vars.SANDBOX_CONTAINER_MAX_INSTANCES }} + VM_AGENT_CONTAINER_MAX_INSTANCES: ${{ vars.VM_AGENT_CONTAINER_MAX_INSTANCES }} + SANDBOX_ENABLED: ${{ vars.SANDBOX_ENABLED }} + MAX_CONCURRENT_SETUP_SESSIONS: ${{ vars.MAX_CONCURRENT_SETUP_SESSIONS }} + SETUP_SESSION_TTL_MS: ${{ vars.SETUP_SESSION_TTL_MS }} + SETUP_SESSION_CAPTURE_POLL_MS: ${{ vars.SETUP_SESSION_CAPTURE_POLL_MS }} + CODEX_DEVICE_AUTH_REQUEST_TIMEOUT_MS: ${{ vars.CODEX_DEVICE_AUTH_REQUEST_TIMEOUT_MS }} + SETUP_SESSION_SWEEP_MAX_CANDIDATES: ${{ vars.SETUP_SESSION_SWEEP_MAX_CANDIDATES }} + POOL_LEASE_BUFFER_MS: ${{ vars.POOL_LEASE_BUFFER_MS }} + SANDBOX_EXEC_TIMEOUT_MS: ${{ vars.SANDBOX_EXEC_TIMEOUT_MS }} + SANDBOX_VM_AGENT_PORT: ${{ vars.SANDBOX_VM_AGENT_PORT }} - name: Re-deploy API Worker (with tail_consumers) if: ${{ inputs.dry_run != true && steps.first_deploy.outputs.is_first == 'true' }} diff --git a/scripts/quality/deploy-reusable-workflow.test.ts b/scripts/quality/deploy-reusable-workflow.test.ts index cf8a7e6640..b95d0e95d3 100644 --- a/scripts/quality/deploy-reusable-workflow.test.ts +++ b/scripts/quality/deploy-reusable-workflow.test.ts @@ -139,22 +139,29 @@ describe('deploy reusable workflow', () => { ); expect(firstDeployResync).toContain('pnpm tsx scripts/deploy/sync-wrangler-config.ts'); - expect(firstDeployResync).toContain('<<: *wrangler_sync_env'); + expect(firstDeployResync).toContain('BASE_DOMAIN: ${{ vars.BASE_DOMAIN }}'); + expect(firstDeployResync).toContain('RESOURCE_PREFIX: ${{ steps.prefix.outputs.value }}'); + expect(firstDeployResync).toContain( + 'ARTIFACTS_BINDING_ENABLED: ${{ vars.ARTIFACTS_BINDING_ENABLED }}' + ); }); it('uses one complete env mapping for every Wrangler config sync invocation', () => { const initialSync = stepBlock('Sync Wrangler Config \\(API \\+ Tail Worker\\)'); const firstDeployResync = stepBlock('Re-sync Wrangler Config \\(add tail_consumers\\)'); - expect(initialSync).toContain('env: &wrangler_sync_env'); - expect(firstDeployResync).toContain('<<: *wrangler_sync_env'); + expect(initialSync).toContain('env:'); + expect(firstDeployResync).toContain('env:'); for (const mapping of Object.values(DIRECT_SYNC_ENV_MAPPINGS)) { expect(initialSync).toContain(mapping); + expect(firstDeployResync).toContain(mapping); } for (const envVar of extractOptionalWorkerEnvVars()) { - expect(initialSync).toContain(`${envVar}: \${{ vars.${envVar} }}`); + const mapping = `${envVar}: \${{ vars.${envVar} }}`; + expect(initialSync).toContain(mapping); + expect(firstDeployResync).toContain(mapping); } }); From bf33e5172d1bcfa6c7c74965eb25f308925d87a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Wed, 5 Aug 2026 22:56:00 +0000 Subject: [PATCH 32/35] fix: reconcile remediation with current deployment config --- ...trap_token_consumes.sql => 0105_bootstrap_token_consumes.sql} | 0 apps/api/wrangler.toml | 1 + 2 files changed, 1 insertion(+) rename apps/api/src/db/migrations/{0101_bootstrap_token_consumes.sql => 0105_bootstrap_token_consumes.sql} (100%) diff --git a/apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql b/apps/api/src/db/migrations/0105_bootstrap_token_consumes.sql similarity index 100% rename from apps/api/src/db/migrations/0101_bootstrap_token_consumes.sql rename to apps/api/src/db/migrations/0105_bootstrap_token_consumes.sql diff --git a/apps/api/wrangler.toml b/apps/api/wrangler.toml index c4a495657d..e60b615629 100644 --- a/apps/api/wrangler.toml +++ b/apps/api/wrangler.toml @@ -368,6 +368,7 @@ crons = ["*/5 * * * *", "30 * * * *", "0 3 * * *", "0 4 * * *", "0 5 1 * *"] # - GITHUB_WEBHOOK_SECRET (optional — overrides ENCRYPTION_KEY for GitHub webhook HMAC) # - DEPLOY_SIGNING_PRIVATE_KEY (Ed25519 signing key for deployment apply payloads) # - DEPLOY_SIGNING_PUBLIC_KEY (Ed25519 verification key for deployment apply payloads) +# - PREVIEW_SIGNING_KEY (HMAC signing key for interactive preview URLs) # - DEVCONTAINER_CACHE_CLOUDFLARE_API_TOKEN (optional — narrower token for managed devcontainer registry credentials) # - DEVCONTAINER_CACHE_CLOUDFLARE_ACCOUNT_ID (optional — account override for managed devcontainer registry credentials) # - GOOGLE_CLIENT_ID (optional — Google Cloud Console OAuth client ID for infra/GCP OIDC integration) From 8e769c3069802ef5a913bb3bd336633b3d9f4390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Thu, 6 Aug 2026 08:14:25 +0000 Subject: [PATCH 33/35] fix: pass deployedMigrationTag in container max_instances tests The #1693 container max_instances tests predate main's #1649 Durable Object migration compatibility work and omitted the required deployedMigrationTag argument, so it arrived as undefined. resolveDurableObjectMigrations treats undefined as an unknown deployed tag and fails closed, making all three tests throw the migration-reconciliation error instead of the expected container validation errors. Pass null (the documented "clean install" value used by every other call site) so the tests exercise container binding validation as intended, without weakening the #1649 fail-closed migration guard. Co-Authored-By: Claude --- scripts/quality/sync-wrangler-config.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/quality/sync-wrangler-config.test.ts b/scripts/quality/sync-wrangler-config.test.ts index 16b5a2de76..819fe002ed 100644 --- a/scripts/quality/sync-wrangler-config.test.ts +++ b/scripts/quality/sync-wrangler-config.test.ts @@ -189,7 +189,7 @@ describe('sync wrangler config', () => { }, ]; - const envConfig = generateApiWorkerEnv({ containers }, outputs, 'prod', false, false); + const envConfig = generateApiWorkerEnv({ containers }, outputs, 'prod', false, false, null); expect(envConfig.containers).toEqual([ { @@ -233,7 +233,7 @@ describe('sync wrangler config', () => { }, ]; - const envConfig = generateApiWorkerEnv({ containers }, outputs, 'prod', false, false); + const envConfig = generateApiWorkerEnv({ containers }, outputs, 'prod', false, false, null); expect(envConfig.containers).toEqual([ { @@ -272,12 +272,12 @@ describe('sync wrangler config', () => { ], }; - expect(() => generateApiWorkerEnv(topLevel, outputs, 'prod', false, false)).toThrow( + expect(() => generateApiWorkerEnv(topLevel, outputs, 'prod', false, false, null)).toThrow( 'SANDBOX_CONTAINER_MAX_INSTANCES must be greater than or equal to 1' ); vi.stubEnv('SANDBOX_CONTAINER_MAX_INSTANCES', '1.5'); - expect(() => generateApiWorkerEnv(topLevel, outputs, 'prod', false, false)).toThrow( + expect(() => generateApiWorkerEnv(topLevel, outputs, 'prod', false, false, null)).toThrow( 'SANDBOX_CONTAINER_MAX_INSTANCES must be a positive safe integer' ); }); From d022d5f9d0b7462992d6f3543850f71306b996f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Thu, 6 Aug 2026 08:52:14 +0000 Subject: [PATCH 34/35] task: add wrangler sync env-parity test coverage gap Follow-up from the PR #1697 cloudflare-specialist review: the deploy-reusable env parity test does not cover DO_MIGRATION_STATE_PROBE_ATTEMPTS or DO_MIGRATION_STATE_PROBE_RETRY_DELAY_MS, because those reach the sync script via readBoundedIntEnv rather than getOptionalProcessEnvVars. Both vars are correctly present in both sync steps today, so this is a missing guardrail rather than a live bug. Co-Authored-By: Claude --- ...ngler-sync-env-parity-test-coverage-gap.md | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 tasks/backlog/2026-08-06-wrangler-sync-env-parity-test-coverage-gap.md diff --git a/tasks/backlog/2026-08-06-wrangler-sync-env-parity-test-coverage-gap.md b/tasks/backlog/2026-08-06-wrangler-sync-env-parity-test-coverage-gap.md new file mode 100644 index 0000000000..628cac9b95 --- /dev/null +++ b/tasks/backlog/2026-08-06-wrangler-sync-env-parity-test-coverage-gap.md @@ -0,0 +1,64 @@ +# Wrangler sync env-parity test misses DO_MIGRATION_STATE_PROBE_* vars + +## Problem + +`scripts/quality/deploy-reusable-workflow.test.ts` enforces that both Wrangler config sync +invocations in `.github/workflows/deploy-reusable.yml` forward an identical env mapping: + +- `Sync Wrangler Config (API + Tail Worker)` +- `Re-sync Wrangler Config (add tail_consumers)` (runs on first deploy, after the tail worker exists) + +The test builds its required list two ways: + +1. `DIRECT_SYNC_ENV_MAPPINGS` — a hand-maintained constant. +2. `extractOptionalWorkerEnvVars()` — dynamically scraped from the `getOptionalProcessEnvVars([...])` + array in `scripts/deploy/sync-wrangler-config.ts`. + +Neither covers `DO_MIGRATION_STATE_PROBE_ATTEMPTS` or `DO_MIGRATION_STATE_PROBE_RETRY_DELAY_MS`, +because those reach the script through a separate `readBoundedIntEnv` call in +`scripts/deploy/durable-object-migrations.ts` rather than through `getOptionalProcessEnvVars`. + +So if a future change drops either var from one of the two sync steps, **no test fails.** + +## Why it matters + +`generateApiWorkerEnv` regenerates `[env.*]` from scratch on every invocation — it does not merge with +prior state. Anything missing from the re-sync step's `process.env` is silently dropped from the +regenerated config on the second `wrangler deploy` of a first-time install. + +Severity is bounded but real: omitting these two falls back to the documented defaults +(`DO_MIGRATION_STATE_PROBE_ATTEMPTS` = 3 attempts, `DO_MIGRATION_STATE_PROBE_RETRY_DELAY_MS` = 2000ms) +rather than bypassing the fail-closed migration-tag check. So it degrades retry tuning, it does not +disable the Durable Object migration safety guard. That is why this is a follow-up and not a blocker. + +This is the same *class* of drift that PR #1697's merge already had to repair by hand: before that +merge, main's re-sync step was missing 26 vars relative to its own primary step, and the PR branch's +re-sync step had independently drifted 10 `PLATFORM_FEEDBACK_*` vars behind its own primary step. +Both drifts existed precisely because the automated parity check did not cover them. The recurring +lesson is that a partially-derived allowlist invites exactly this drift. + +## Context + +Discovered by the `cloudflare-specialist` review during PR #1697 (merge of `origin/main` into the +strict-CTO remediation bundle, 2026-08-06). At that time both vars **are** correctly present in both +steps — verified by direct grep — so there is no live bug today, only a missing guardrail. + +## Acceptance Criteria + +- [ ] The parity test covers `DO_MIGRATION_STATE_PROBE_ATTEMPTS` and + `DO_MIGRATION_STATE_PROBE_RETRY_DELAY_MS`, either by extending `DIRECT_SYNC_ENV_MAPPINGS` or by + also scraping `readBoundedIntEnv` call sites in `scripts/deploy/durable-object-migrations.ts`. +- [ ] Prefer a derivation that cannot silently miss a *future* var read through a third mechanism — + e.g. scrape every `process.env.X` read reachable from `sync-wrangler-config.ts` and assert each + appears in both steps, rather than maintaining another hand-written allowlist. +- [ ] The test is proven discriminating: temporarily delete one of the two vars from the re-sync step + and confirm the test goes red before relying on it. +- [ ] No production behavior change — this is test-coverage hardening only. + +## References + +- `scripts/quality/deploy-reusable-workflow.test.ts` (`uses one complete env mapping for every + Wrangler config sync invocation`) +- `scripts/deploy/durable-object-migrations.ts` (`readBoundedIntEnv`) +- `.claude/rules/07-env-and-urls.md` — Wrangler binding + DO migration safety rules +- PR #1697; main's DO-migration-compat work in #1649 From da9e1b6413936ae8e8077c2c8a2d00da97ba51ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Thu, 6 Aug 2026 09:24:12 +0000 Subject: [PATCH 35/35] chore: retrigger CI with fresh PR payload after label removal The Specialist Review Evidence check reads pull_request.labels from the workflow event payload. `gh run rerun` replays the original payload, which still carried the needs-human-review label, so the re-run kept failing after the label was removed. The CI workflow only listens to the default pull_request types (opened/synchronize/reopened), so an unlabeled event does not start a new run. This empty commit produces a synchronize event with the current label set. Co-Authored-By: Claude