Superseded: wrong follow-up branch - #898
kentcdodds wants to merge 38 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds a production D1 backup control plane with immutable R2 storage, signed manifests, scheduled workflows, resource provisioning, isolated restore-drill tooling, canonical readiness assessment, extensive tests, and disaster-recovery documentation. ChangesBackup control plane
Backup resource provisioning
D1 restore drill
Canonical readiness
Disaster-recovery documentation
Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant BackupWorkflow
participant D1
participant R2
Scheduler->>BackupWorkflow: enqueue scheduled backup
BackupWorkflow->>D1: verify identity and start export
D1-->>BackupWorkflow: bookmark and signed download URL
BackupWorkflow->>D1: poll until export completes
BackupWorkflow->>R2: store immutable SQL object
BackupWorkflow->>R2: write signed manifest
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
tools/disaster-recovery/d1-restore-drill-cli.ts (1)
295-302: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDecode child output on stream boundaries, not per-chunk.
chunk.toString()decodes each Buffer independently, so a multibyte UTF-8 sequence split across two chunks becomes replacement characters. Wrangler D1--jsonoutput can carry non-ASCII schema/primary-key values, so this can spuriously corruptparseQueryRows/verifyRows. UsesetEncoding('utf8')(which uses aStringDecoderthat handles boundaries) or buffer then decode once.♻️ Proposed fix using StringDecoder-backed encoding
let stdout = '' let stderr = '' + child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') - child.stdout.on('data', (chunk: Buffer) => { - stdout += chunk.toString() + child.stdout.on('data', (chunk: string) => { + stdout += chunk }) - child.stderr.on('data', (chunk: Buffer) => { - stderr += chunk.toString() + child.stderr.on('data', (chunk: string) => { + stderr += chunk })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/disaster-recovery/d1-restore-drill-cli.ts` around lines 295 - 302, Update the child process output handling around stdout and stderr to decode UTF-8 across stream boundaries, using setEncoding('utf8') before registering data listeners or buffering raw chunks and decoding once after completion. Preserve the existing stdout/stderr accumulation consumed by parseQueryRows and verifyRows, ensuring split multibyte characters are not corrupted.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/contributing/setup-manifest.md`:
- Around line 112-116: Update the backup deployment sentence in the setup
manifest to replace “requires reviewed non-secret” with “requires the following
reviewed non-secret” before listing the variables, preserving the remainder of
the key and secret guidance.
- Around line 81-85: Update the production backup documentation near the
BACKUP_BUCKET description to add or link to a post-provisioning check confirming
the R2 bucket has no public r2.dev endpoint or custom-domain exposure. Keep the
existing private-binding and provisioner-scope details, and identify the
verification procedure clearly for operators.
- Around line 90-97: Update the backup-resources CLI example in
setup-manifest.md to match the canonical command in disaster-recovery.md,
including the provisioner token source, explicit --bucket-name, and
--worker-name options; alternatively replace the snippet with a direct link to
that canonical example.
In `@package.json`:
- Around line 51-55: Update the package.json validate script to include the
backup control-plane test target alongside the existing worker test command,
ensuring packages/backup-control-plane/*.node.test.ts runs as part of the
authoritative validation gate. Add the corresponding backup test command to the
concurrently-managed validation tasks and names without removing existing
checks.
In `@packages/backup-control-plane/d1-export-api.ts`:
- Around line 203-244: Update parseExportState to recognize the D1 export API’s
valid non-terminal in-progress status values as pending instead of throwing
export-malformed-response. Keep the existing error and complete handling
unchanged, and retain malformed-response validation for unknown statuses; use
the API’s actual pending status values rather than accepting arbitrary strings.
In `@packages/backup-control-plane/manifest-signing.node.test.ts`:
- Around line 81-100: Update the tamperedSignature setup in the signature
verification test to create an independent copy of signature before mutating its
first byte. Preserve the original signature for the subsequent
wrongKeys.publicKey verification assertion.
---
Nitpick comments:
In `@tools/disaster-recovery/d1-restore-drill-cli.ts`:
- Around line 295-302: Update the child process output handling around stdout
and stderr to decode UTF-8 across stream boundaries, using setEncoding('utf8')
before registering data listeners or buffering raw chunks and decoding once
after completion. Preserve the existing stdout/stderr accumulation consumed by
parseQueryRows and verifyRows, ensuring split multibyte characters are not
corrupted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bf9d092d-3448-4eff-9ccc-f23c6225b6bb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (59)
docs/contributing/architecture/primitives.yamldocs/contributing/disaster-recovery.mddocs/contributing/environment-variables.mddocs/contributing/index.mddocs/contributing/setup-manifest.mdpackage.jsonpackages/backup-control-plane/backup-control-plane-test-support.tspackages/backup-control-plane/backup-policy.node.test.tspackages/backup-control-plane/backup-policy.tspackages/backup-control-plane/backup-runtime.node.test.tspackages/backup-control-plane/backup-runtime.tspackages/backup-control-plane/backup-types.tspackages/backup-control-plane/backup-workflow.tspackages/backup-control-plane/d1-export-api.node.test.tspackages/backup-control-plane/d1-export-api.tspackages/backup-control-plane/durable-export.node.test.tspackages/backup-control-plane/durable-export.tspackages/backup-control-plane/freshness-check.node.test.tspackages/backup-control-plane/freshness-check.tspackages/backup-control-plane/freshness-retry.node.test.tspackages/backup-control-plane/freshness-retry.tspackages/backup-control-plane/immutable-storage.node.test.tspackages/backup-control-plane/immutable-storage.tspackages/backup-control-plane/manifest-signing.node.test.tspackages/backup-control-plane/manifest-signing.tspackages/backup-control-plane/package.jsonpackages/backup-control-plane/readme.mdpackages/backup-control-plane/tsconfig.jsonpackages/backup-control-plane/vitest.config.tspackages/backup-control-plane/worker.tspackages/backup-control-plane/workflow-trigger.node.test.tspackages/backup-control-plane/workflow-trigger.tspackages/backup-control-plane/wrangler.jsoncpackages/shared/src/backup-manifest.tstools/ci/backup-resources-cli.tstools/ci/backup-resources.node.test.tstools/ci/backup-resources.tstools/disaster-recovery/canonical-json.node.test.tstools/disaster-recovery/canonical-json.tstools/disaster-recovery/canonical-readiness-cli.tstools/disaster-recovery/canonical-readiness-signatures.node.test.tstools/disaster-recovery/canonical-readiness.tstools/disaster-recovery/d1-restore-drill-cli.tstools/disaster-recovery/d1-restore-drill.tstools/disaster-recovery/disaster-recovery-test-support.tstools/disaster-recovery/readiness-assessment.tstools/disaster-recovery/readiness-contracts.tstools/disaster-recovery/readiness-evidence-schema.tstools/disaster-recovery/readiness-validation.tstools/disaster-recovery/readme.mdtools/disaster-recovery/restore-cli-and-staging.node.test.tstools/disaster-recovery/restore-drill-execution.node.test.tstools/disaster-recovery/restore-trust-and-verification.node.test.tstools/disaster-recovery/restore-trust.tstools/disaster-recovery/restore-wrangler-contract.node.test.tstools/disaster-recovery/trusted-backup-manifest-public-keys.jsontools/disaster-recovery/trusted-d1-restore-identities.jsontools/disaster-recovery/trusted-readiness-public-keys.jsontools/disaster-recovery/trusted-restore-baselines.json
| Production backups use a separate deployment and are not provisioned or deployed | ||
| by the application workflow above. The dedicated Worker and Workflow live under | ||
| `packages/backup-control-plane/` in an independently administered DR Cloudflare | ||
| account. Its `BACKUP_BUCKET` R2 binding is private and uses immutable `daily/` | ||
| and `weekly/` prefixes. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Document the required public-access check.
A private BACKUP_BUCKET Worker binding does not prove that the R2 bucket has no r2.dev or custom-domain exposure. The runbook explicitly says the provisioner does not manage those APIs, so add the post-provisioning verification here or link to it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/contributing/setup-manifest.md` around lines 81 - 85, Update the
production backup documentation near the BACKUP_BUCKET description to add or
link to a post-provisioning check confirming the R2 bucket has no public r2.dev
endpoint or custom-domain exposure. Keep the existing private-binding and
provisioner-scope details, and identify the verification procedure clearly for
operators.
| ```sh | ||
| node tools/ci/backup-resources-cli.ts plan \ | ||
| --source-account-id "<PRODUCTION_ACCOUNT_ID>" \ | ||
| --destination-account-id "<DR_ACCOUNT_ID>" \ | ||
| --source-d1 "<PRODUCTION_D1_UUID>:kody" \ | ||
| --deny-production-resource kody-email-blobs \ | ||
| --deny-production-resource kody-community-assets | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Finding relevant files..."
git ls-files | rg '(^docs/contributing/setup-manifest\.md$|^docs/.*/disaster-recovery\.md$|backup-resources-cli\.ts$|disaster-recovery\.md$)' || true
echo
echo "Relevant snippet in setup-manifest.md:"
sed -n '70,125p' docs/contributing/setup-manifest.md || true
echo
echo "Search for backup-resources-cli usages and docs:"
rg -n "backup-resources-cli|backup-resources|source-d1|destination-account-id|bucket-name|worker-name|provisioner|provide.*token" docs disaster-recovery setup-manifest tools packages 2>/dev/null || trueRepository: kentcdodds/kody
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "setup-manifest relevant section:"
sed -n '1,150p' docs/contributing/setup-manifest.md | nl -ba | sed -n '70,130p'
echo
echo "Exact relevant flag usages in tracked text files (narrow):"
rg -n --glob '!*.{min.js,js.map,tsbuildinfo,min.css}' --glob '*.md' --glob '*.ts' --glob '*.tsx' \
"backup-resources-cli|backup-resources|plan.*--bucket-name|plan.*--worker-name|supplying|provisioner|PROVISION|token" \
docs disaster-recovery README packages tools 2>/dev/null || true
echo
echo "Locate backup-resources-cli implementation:"
fd -a 'backup-resources-cli\.ts$' . 2>/dev/null || trueRepository: kentcdodds/kody
Length of output: 226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "setup-manifest relevant section:"
sed -n '70,130p' docs/contributing/setup-manifest.md
echo
echo "backup CLI related occurrences in repository text files:"
rg -n --glob '!*.{min.js,js.map,tsbuildinfo,min.css}' --glob '*.md' --glob '*.ts' --glob '*.tsx' \
"backup-resources-cli|backup-resources|\\-\\-bucket-name|\\-\\-worker-name|provisioner|PROVISION|plan" \
docs packages tools disaster-recovery 2>/dev/null || true
echo
echo "Locate backup-resources-cli implementation:"
fd -a 'backup-resources-cli\.ts$' . 2>/dev/null || find . -name 'backup-resources-cli.ts' -printRepository: kentcdodds/kody
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "tools/ci/backup-resources-cli.ts argument parsing:"
sed -n '1,170p' tools/ci/backup-resources-cli.ts
echo
echo "tools/ci/backup-resources.ts resource construction relevant sections:"
sed -n '1,160p' tools/ci/backup-resources.ts
sed -n '480,550p' tools/ci/backup-resources.ts
echo
echo "disaster-recovery canonical example around plan/apply:"
sed -n '240,290p' docs/contributing/disaster-recovery.md
echo
echo "backup control plane wrangler constants for bucket/worker if present:"
rg -n "DR_R2_PROVISIONER_TOKEN|BACKUP_BUCKET|backup-resources-cli|kody-production-d1-backups|bucket-name|worker-name" packages/backup-control-plane docs/contributing/disaster-recovery.md tools/ci -g '!*.{min.js,js.map,tsbuildinfo}' || trueRepository: kentcdodds/kody
Length of output: 16672
Copy the canonical backup resources example.
The setup-manifest snippet omits the provisioner token source plus the explicit --bucket-name and --worker-name used by the operator. Add the same full command shown in disaster-recovery.md or link directly to it so the plan cannot be copied as runnable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/contributing/setup-manifest.md` around lines 90 - 97, Update the
backup-resources CLI example in setup-manifest.md to match the canonical command
in disaster-recovery.md, including the provisioner token source, explicit
--bucket-name, and --worker-name options; alternatively replace the snippet with
a direct link to that canonical example.
| The backup deployment also requires reviewed non-secret | ||
| `BACKUP_MANIFEST_SIGNING_KEY_ID`, `TRUSTED_RESTORE_BASELINE_ID`, and | ||
| `TRUSTED_RESTORE_BASELINE_SHA256` vars. Store the matching base64-encoded | ||
| Ed25519 PKCS#8 private key only as the | ||
| `BACKUP_MANIFEST_SIGNING_PRIVATE_KEY_PKCS8_BASE64` Worker secret. Never commit |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the sentence grammar.
Change “requires reviewed non-secret” to “requires the following reviewed non-secret” (or “requires review of the following non-secret”).
🧰 Tools
🪛 LanguageTool
[style] ~112-~112: The double modal “requires reviewed” is nonstandard (only accepted in certain dialects). Consider “to be reviewed”.
Context: ...s. The backup deployment also requires reviewed non-secret `BACKUP_MANIFEST_SIGNING_KEY...
(NEEDS_FIXED)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/contributing/setup-manifest.md` around lines 112 - 116, Update the
backup deployment sentence in the setup manifest to replace “requires reviewed
non-secret” with “requires the following reviewed non-secret” before listing the
variables, preserving the remainder of the key and secret guidance.
Source: Linters/SAST tools
| "typecheck": "nx run worker:typecheck && tsc --noEmit -p packages/backup-control-plane/tsconfig.json", | ||
| "test": "nx run worker:test", | ||
| "test:push": "npm run test && npm run test:e2e:run", | ||
| "inspect": "npx -y @mcpjam/inspector inspector", | ||
| "validate": "concurrently -n format,lint,typecheck,test,e2e,mcp,primitives,migrations -c green,yellow,magenta,blue,cyan,red,white,gray \"npm run format:check\" \"npm run lint\" \"npm run typecheck\" \"npm run test\" \"npm run test:e2e:run\" \"npm run test:mcp\" \"npm run primitives:check\" \"npm run migrations:check\"", | ||
| "validate": "concurrently -n format,lint,typecheck,test,e2e,mcp,backup-build,primitives,migrations -c green,yellow,magenta,blue,cyan,red,blueBright,white,gray \"npm run format:check\" \"npm run lint\" \"npm run typecheck\" \"npm run test\" \"npm run test:e2e:run\" \"npm run test:mcp\" \"npm run backup:build\" \"npm run primitives:check\" \"npm run migrations:check\"", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'nx.json'
fd -t f 'project.json' packages/backup-control-plane packages/worker 2>/dev/null
cat packages/backup-control-plane/package.json 2>/dev/null
cat packages/backup-control-plane/vitest.config.ts 2>/dev/null
fd -t f -e yml -e yaml . .github/workflows 2>/dev/null --exec sh -c 'echo "== {} =="; cat {}'
rg -n 'backup-control-plane|disaster-recovery|node\.test' package.json nx.json 2>/dev/nullRepository: kentcdodds/kody
Length of output: 44530
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -t f 'package.json|project.json|nx.json|vitest.config.ts|vitest.config.*|tsconfig.json|tsconfig.*' \
packages/worker packages/backup-control-plane tools/disaster-recovery 2>/dev/null || true
echo
echo "== package.json scripts =="
node - <<'JS'
const fs = require('node:fs');
const p = JSON.parse(fs.readFileSync('package.json','utf8'));
for (const [k,v] of Object.entries(p.scripts || {})) {
if (/^(test|typecheck|backup|validate|migrations|primitives)|backup|disaster-recovery/.test(k)) {
console.log(`${k}: ${v}`);
}
}
JS
echo
echo "== nx/workspace refs =="
rg -n '"workspace|apps|projects|implicitDeps|targetDefaults|backup-control-plane|worker:test|backup-control-plane:test|test":' package.json nx.json packages packages/backup-control-plane/project.json packages/worker/project.json 2>/dev/null || true
echo
echo "== disaster-recovery config/package files =="
fd -t f -e ts . tools/disaster-recovery 2>/dev/null --exec sh -c 'echo "-- {} --"; wc -l "{}"; sed -n "1,220p" "{}"'Repository: kentcdodds/kody
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== scripts of interest =="
sed -n '1,140p' package.json
echo
echo "== project files =="
for f in nx.json packages/worker/project.json packages/backup-control-plane/project.json; do
echo "--- $f ---"
sed -n '1,220p' "$f"
done
echo
echo "== validate workflow direct call =="
sed -n '1,130p' .github/workflows/validate.yml
echo
echo "== exact references to backup-control-plane and worker:test =="
rg -n 'backup-control-plane|worker:test|backup-control-plane:test|validate:|concurrently|npm run validate|test":' package.json package-lock.json .github/workflows packages . -g '!node_modules' -g '!dist' -g '!build' 2>/dev/null | sed -n '1,260p'Repository: kentcdodds/kody
Length of output: 10625
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== worker/project vitest config files =="
fd -t f 'vitest.config.*|vite.config.*|package.json|project.json' packages/worker -x sh -c 'echo "--- $1 ---"; sed -n "1,220p" "$1"' sh {}
echo
echo "== backup-control-plane project/package/vitest files =="
fd -t f 'vitest.config.*|vite.config.*|package.json|project.json' packages/backup-control-plane -x sh -c 'echo "--- $1 ---"; sed -n "1,220p" "$1"' sh {}
echo
echo "== disaster-recovery test/vitest files only =="
fd -t f -i 'vitest.config.*|*.test.ts|*.spec.ts' tools/disaster-recovery packages/backup-control-plane 2>/dev/null -x sh -c 'echo "--- $1 ---"; sed -n "1,220p" "$1"' sh {}
echo
echo "== precise validate script decomposition =="
python3 - <<'PY'
import json
with open('package.json') as f:
p=json.load(f)
scripts=[]
names=[]
for cmd in p['scripts']['validate'].split('" "',):
s=cmd.strip('"')
cmd=s.split(' ')[-1].split(' ')[0]
scripts.append(cmd)
names.append(s.split(' ')[-1].replace('npm run ','').replace('\"',''))
print('\n'.join(f'{k}: {v}' for k,v in zip(names, scripts)))
print('validate_names_count=', len(names))
print('validate_script_count=', len(scripts))
PYRepository: kentcdodds/kody
Length of output: 5577
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all top-level vitest config files =="
fd -t f 'vitest.config.*' . -maxdepth 2 -x sh -c 'echo "--- $1 ---"; sed -n "1,260p" "$1"' sh {}
echo
echo "== packages containing vitest project declarations or configs =="
rg -n 'node-unit|workers-unit|mcp-e2e|defineConfig|projects:' packages -g '*vitest*' -g '*test*' -g '!node_modules' | sed -n '1,320p'
echo
echo "== disaster-recovery test files without project config text =="
fd -t f -i '.*\.node\.test\.ts|.*\.node\.spec\.ts|vitest.*config.*' tools/disaster-recovery packages/backup-control-plane 2>/dev/null -x sh -c 'echo "--- $1 ---"; wc -l "$1"; sed -n "1,160p" "$1"' sh {}
echo
echo "== exact validate invocation in CI =="
sed -n '1,130p' .github/workflows/validate.yml 2>/dev/null || trueRepository: kentcdodds/kody
Length of output: 354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all top-level vitest config files =="
fd 'vitest.config\.' . -maxdepth 2 -x sh -c 'echo "--- $1 ---"; sed -n "1,260p" "$1"' sh {}
echo
echo "== packages containing vitest project declarations or configs =="
rg -n 'node-unit|workers-unit|mcp-e2e|defineConfig|projects:' packages -g '*vitest*' -g '*test*' -g '!node_modules' | sed -n '1,320p'
echo
echo "== disaster-recovery test files without project config text =="
fd '.*\.node\.test\.ts|.*\.node\.spec\.ts|vitest.*config.*' tools/disaster-recovery packages/backup-control-plane 2>/dev/null -x sh -c 'echo "--- $1 ---"; wc -l "$1"; sed -n "1,160p" "$1"' sh {}
echo
echo "== exact validate invocation in CI =="
sed -n '1,130p' .github/workflows/validate.yml 2>/dev/null || trueRepository: kentcdodds/kody
Length of output: 354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all top-level vitest config files =="
find . -maxdepth 2 -type f -path '*vitest.config.*' -print -exec sh -c 'echo "--- $1 ---"; sed -n "1,260p" "$1"' sh {} \;
echo
echo "== packages containing vitest project declarations or configs =="
rg -n 'node-unit|workers-unit|mcp-e2e|defineConfig|projects:' packages -g '*vitest*' -g '*test*' -g '!node_modules' | sed -n '1,320p'
echo
echo "== disaster-recovery test files with simple names =="
find tools/disaster-recovery -type f \( -name '*.node.test.ts' -o -name '*.node.spec.ts' -path '*vitest*' \) -print -exec sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,160p" "$1"' sh {} \;
echo
echo "== exact validate invocation in CI =="
sed -n '1,130p' .github/workflows/validate.yml 2>/dev/null || trueRepository: kentcdodds/kody
Length of output: 25778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== backup-control-plane test files =="
find packages/backup-control-plane -type f -name '*.node.test.ts' -print | sort
echo
echo "== worker vitest config files inside packages/worker =="
find packages/worker -type f -path '*vitest*' -print | sort -u
for f in $(find packages/worker -type f -path '*vitest*' | sort -u); do
echo "--- $f ---"
sed -n '1,260p' "$f"
done
echo
echo "== package.json validate decomposition =="
node - <<'JS'
const fs = require('node:fs');
const p = JSON.parse(fs.readFileSync('package.json','utf8'));
const validate = p.scripts.validate;
const commands = validate.match(/"[^"]+"/g).map(s => s.slice(1, -1).split(' ').at(-1));
const names = validate.split('" "')
.filter(Boolean)
.map(part => part.replace(/^"|"$/, '')
.replace(/npm run[ ]+/, '')
.split(' ')[0]);
console.log('commands=', commands.join(' | '));
console.log('names=', names.join(' | '));
JSRepository: kentcdodds/kody
Length of output: 1073
Include the backup/DR test script in npm run validate.
validate runs test (nx run worker:test), but run worker:test only executes the configured Vitest projects in packages/worker. It does not reach packages/backup-control_plane/*.node.test.ts; those only run with packages/backup-control-plane:test, which is not part of validate or CI’s authoriative gate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 51 - 55, Update the package.json validate script
to include the backup control-plane test target alongside the existing worker
test command, ensuring packages/backup-control-plane/*.node.test.ts runs as part
of the authoritative validation gate. Add the corresponding backup test command
to the concurrently-managed validation tasks and names without removing existing
checks.
Source: Coding guidelines
| function parseExportState(result: JsonObject): ExportState { | ||
| if ( | ||
| result.type !== 'export' || | ||
| result.success !== true || | ||
| typeof result.at_bookmark !== 'string' || | ||
| result.at_bookmark.length === 0 | ||
| ) { | ||
| throw new BackupError( | ||
| 'export-malformed-response', | ||
| 'D1 export response was invalid', | ||
| ) | ||
| } | ||
| const status = result.status | ||
| if (status === 'error') { | ||
| throw new BackupError('export-failed', 'D1 reported an export failure') | ||
| } | ||
| if (status === 'complete') { | ||
| const completed = result.result | ||
| if ( | ||
| !isObject(completed) || | ||
| typeof completed.signed_url !== 'string' || | ||
| completed.signed_url.length === 0 | ||
| ) { | ||
| throw new BackupError( | ||
| 'export-malformed-response', | ||
| 'D1 completed without a signed URL', | ||
| ) | ||
| } | ||
| return { | ||
| kind: 'complete', | ||
| bookmark: result.at_bookmark, | ||
| signedUrl: completed.signed_url, | ||
| } | ||
| } | ||
| if (status !== undefined) { | ||
| throw new BackupError( | ||
| 'export-malformed-response', | ||
| 'D1 export returned an unknown status', | ||
| ) | ||
| } | ||
| return { kind: 'pending', bookmark: result.at_bookmark } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg 'packages/backup-control-plane/(d1-export-api\.ts|.*test.*|.*support.*)' || true
echo
echo "== d1-export-api outline/sections =="
wc -l packages/backup-control-plane/d1-export-api.ts
sed -n '1,290p' packages/backup-control-plane/d1-export-api.ts
echo
echo "== usages of parseExportState/startD1Export/pollD1Export =="
rg -n "parseExportState|startD1Export|pollD1Export|polling|signed_url|at_bookmark|status" packages/backup-control-plane -SRepository: kentcdodds/kody
Length of output: 18727
🌐 Web query:
Cloudflare D1 export Polling API status field active complete error output_format polling
💡 Result:
When performing a Cloudflare D1 database export using the output_format: "polling" parameter, the API returns a status field in the response [1][2]. While the documentation explicitly identifies "complete" and "error" as specific values for this field [1][3], any response that is not marked as "complete" or "error" indicates that the export process is still in progress (often implicitly treated as "active" or "pending") [4]. Key details regarding this polling mechanism include: 1. Polling Status: You must continue to poll the export endpoint until the status field returns either "complete" or "error" [1][5]. If an in-progress export is not continually polled, it will automatically cancel to avoid blocking the database [5][6]. 2. Error Handling: The error field is only populated when status is set to "error" and will contain the associated error message [1][2]. 3. Completion: Once status reaches "complete", the result object will be present, containing the filename and a signed_url for downloading the exported SQL file [1][3]. 4. State Management: The at_bookmark value returned in the response should be provided in subsequent requests to maintain the state of the in-progress export [1][2]. Implementation note: If the API returns a status other than "complete" or "error", the process is ongoing, and you should continue polling with the updated at_bookmark [4].
Citations:
- 1: https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export
- 2: https://developers.cloudflare.com/api/python/resources/d1/subresources/database/methods/export/
- 3: https://developers.cloudflare.com/api/node/resources/d1/
- 4: https://github.com/cloudflare/workers-sdk/blob/e643b19d/packages/wrangler/src/d1/export.ts
- 5: https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export/
- 6: https://developers.cloudflare.com/api/go/resources/d1/subresources/database/methods/export/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fetch Cloudflare D1 export polling docs example/status values =="
curl -fsSL 'https://raw.githubusercontent.com/cloudflare/api-snapshots/main/packages/d1/snapshots/responses/d1_database_export.json' || true
echo
echo "== fetch Cloudflare API docs page relevant polling examples (raw HTML snippets if available via docs mirror / JSON docs, otherwise HTML) =="
curl -fsSL 'https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export' | sed -n '/status/,/at_bookmark/p' | head -200 || true
echo
echo "== inspect wrangler D1 export poll implementation if vendored in repo =="
rg -n 'status:"active"|status===.active.|is complete|status.*active|export polling' . -S -g '!node_modules' -g '!dist' -g '!build' | head -200Repository: kentcdodds/kody
Length of output: 50372
🌐 Web query:
Cloudflare D1 polling exports status active documentation
💡 Result:
When exporting a Cloudflare D1 database via the REST API using the polling method, the process is asynchronous [1]. Because generating a SQL dump for larger databases takes time—during which the database is unavailable for queries—the API requires continuous polling to prevent the export task from automatically canceling [2][3][4]. To manage this process, you must use the following workflow: 1. Initiation: Send a POST request to the D1 export endpoint with output_format: "polling" [2][3][4]. The response will include a status and an at_bookmark string [2][5][4]. 2. Polling: Periodically send subsequent POST requests to the same endpoint, providing the current_bookmark returned by the previous response [2][6][4]. 3. Completion: Continue polling until the status field in the response changes to "complete" [2][1][4]. Once complete, the response will include a signed_url which provides a temporary (one-hour) link to download the generated SQL file, and a filename [2][7][3]. If the status returns "error", the response will include an error field containing the failure message [2][4]. It is important to note that keeping the export active requires regular polling, as the system will cancel the job if it detects that the client has stopped requesting status updates [2][1][4].
Citations:
- 1: https://takumidev.tech/querying-exporting-cloudflare-d1-with-go/
- 2: https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export/
- 3: https://developers.cloudflare.com/api/resources/d1/
- 4: https://developers.cloudflare.com/api/go/resources/d1/subresources/database/methods/export/
- 5: https://developers.cloudflare.com/api/python/resources/d1/subresources/database/methods/export/
- 6: https://developers.cloudflare.com/api/node/resources/d1/
- 7: https://developers.cloudflare.com/api/node/resources/d1/subresources/database/methods/export/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
html = open('/tmp/coderabbit-shell-logs/shell-output-fEPid3.txt', encoding='utf-8', errors='ignore').read()
needle = 'D1 database export'
idx = html.find(needle)
print('needle_idx=' + str(idx))
if idx != -1:
sample = html[idx:idx+10000]
for pat in ['status', 'active', 'Complete', 'Export', 'Polling', 'completed', 'at_bookmark']:
print(pat + ':', sample.count(pat))
print('\n--- status nearby context ---')
for m in re.finditer(r'status', sample):
lo, hi = max(0,m.start()-500), min(len(sample),m.end()+1500)
print(sample[lo:hi])
print('---')
PYRepository: kentcdodds/kody
Length of output: 335
🌐 Web query:
site:developers.cloudflare.com/api/resources/d1/subresources/database/methods/export/export polling active
💡 Result:
Cloudflare D1 export supports a polling mode: set output_format: "polling" and, for an in-progress export, pass current_bookmark from the previous polling response. The docs say the export must be continually polled or it will automatically cancel. (developers.cloudflare.com)
The polling response includes at_bookmark plus messages, and when finished it returns status: "complete" with a signed_url for the SQL download (valid for one hour). (developers.cloudflare.com)
Citations:
- 1: https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export/?utm_source=openai
- 2: https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export/?utm_source=openai
🌐 Web query:
Cloudflare D1 polling export response status active messages current_bookmark
💡 Result:
To export a Cloudflare D1 database using the "polling" output format, you interact with the D1 export API [1][2]. This process involves an initial request to start the export and subsequent polling requests to monitor the status until completion [3]. Key components of the polling mechanism: 1. Initial Request: Call the export endpoint with output_format: "polling" [1][3]. The API will return an at_bookmark value, which acts as a persistent identifier for that specific export task [1][2]. 2. Polling: To check the status of an in-progress export, send a follow-up request to the same endpoint including the current_bookmark parameter, set to the value of the at_bookmark received from your initial request [1][4][2]. 3. Response Fields: - status: Indicates the current state of the export, either "complete" or "error" [1][2]. - messages: An array of strings providing logs or updates since the last time you polled [1][2]. - at_bookmark: The time-travel bookmark associated with the export task; this remains constant for the duration of the process [1][2]. - result: When status is "complete", this object will contain the generated SQL filename and a signed_url valid for one hour to download the exported data [1][2]. - error: Only populated if status is "error", containing the relevant error message [1][2]. Note: Larger databases may be unavailable for queries during the export process [2][5]. To avoid blocking the database unnecessarily, ensure that in-progress exports are continually polled; otherwise, the task may automatically cancel [2][5].
Citations:
- 1: https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export
- 2: https://developers.cloudflare.com/api/resources/d1/subresources/database/methods/export/
- 3: https://developers.cloudflare.com/workflows/examples/backup-d1/
- 4: https://developers.cloudflare.com/api/node/resources/d1/
- 5: https://developers.cloudflare.com/api/resources/d1/
Handle non-terminal D1 export statuses as pending.
parseExportState only accepts status: for complete and error, and throws export-malformed-response for any other status value. The D1 polling response can also include a status for in-progress exports, so move status === undefined into an explicit pending branch or document/validate the real in-progress status before failing malformed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backup-control-plane/d1-export-api.ts` around lines 203 - 244,
Update parseExportState to recognize the D1 export API’s valid non-terminal
in-progress status values as pending instead of throwing
export-malformed-response. Keep the existing error and complete handling
unchanged, and retain malformed-response validation for unknown statuses; use
the API’s actual pending status values rather than accepting arbitrary strings.
| const tamperedSignature = signature.slice() | ||
| tamperedSignature[0] = (tamperedSignature[0] ?? 0) ^ 1 | ||
| assert.equal( | ||
| await crypto.subtle.verify( | ||
| 'Ed25519', | ||
| await importPublicKey(signingKeys.publicKey), | ||
| tamperedSignature, | ||
| payloadBytes, | ||
| ), | ||
| false, | ||
| ) | ||
| assert.equal( | ||
| await crypto.subtle.verify( | ||
| 'Ed25519', | ||
| await importPublicKey(wrongKeys.publicKey), | ||
| signature, | ||
| payloadBytes, | ||
| ), | ||
| false, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node -e 'const b=Buffer.from([1,2,3]);const s=b.slice();s[0]=9;console.log("slice shares:",b[0]===9);const c=Buffer.from(b);c[0]=5;console.log("from copies:",b[0]!==5)'Repository: kentcdodds/kody
Length of output: 190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the referenced test without executing repo code.
if [ -f packages/backup-control-plane/manifest-signing.node.test.ts ]; then
sed -n '1,130p' packages/backup-control-plane/manifest-signing.node.test.ts | cat -n
else
fd -a 'manifest-signing.node.test.ts' .
fiRepository: kentcdodds/kody
Length of output: 4226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe using only Node Buffer/web crypto APIs, without running repo code.
node - <<'JS'
const keys = crypto.subtle.generateKeySync('Ed25519', true, ['sign', 'verify'])
const payload = new TextEncoder().encode('payload')
const wrong = crypto.subtle.generateKeySync('Ed25519', true, ['verify'])
let exported
crypto.subtle.exportKey('jwk', keys.publicKey, { format: 'jwk' }).then(jwk => {
exported = jwk
return exported
})
.then(async jwk => {
const signer = await crypto.subtle.importKey('jwk', keys.privateKey, 'Ed25519', true, ['sign'])
const public = await crypto.subtle.importKey('jwk', jwk, 'Ed25519', true, ['verify'])
const wrongPublic = await crypto.subtle.importKey('jwk', wrong.publicKey, 'Ed25519', true, ['verify'])
const sigArray = await crypto.subtle.sign('Ed25519', signer, payload)
const sig = Buffer.from(sigArray)
console.log('original-signing-key', await crypto.subtle.verify('Ed25519', public, sig, payload))
const tamperedSignature = sig.slice()
tamperedSignature[0] = (tamperedSignature[0] ?? 0) ^ 1
console.log('same-memory-after-tamper', sig[0] === tamperedSignature[0])
console.log('tampered-signing-key-rejected', await crypto.subtle.verify('Ed25519', public, tamperedSignature, payload))
console.log('original-wrong-key-would-match', sig[0] !== (tamperedSignature[0] ?? 0) ^ 1 ? 'NA' : 'original is now invalid due to slice sharing')
console.log('tampered-wrong-key-rejected', await crypto.subtle.verify('Ed25519', correctWrongPublic = await crypto.subtle.importKey('jwk', wrong.publicKey, 'Ed25519', true, ['verify']), tamperedSignature, payload))
})
JSRepository: kentcdodds/kody
Length of output: 838
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe using available Node crypto APIs; this proves Buffer.slice sharing and the signature mutation sequence.
node - <<'JS'
const { generateKeyPairSync } = require('node:crypto')
const keys = generateKeyPairSync('Ed25519')
const wrong = generateKeyPairSync('Ed25519')
async function importPublicKey(key) {
const der = key.export({ format: 'der', type: 'spki' })
return crypto.subtle.importKey('spki', der, 'Ed25519', false, ['verify'])
}
const payload = 'payload'
const payloadBytes = new TextEncoder().encode(payload)
const signer = await crypto.subtle.importKey(
'pkcs8',
Buffer.from(keys.privateKey.export({ format: 'der', type: 'pkcs8' })),
'Ed25519',
false,
['sign']
)
const signatureArray = await crypto.subtle.sign('Ed25519', signer, payloadBytes)
const signature = Buffer.from(signatureArray)
console.log('signature-valid-with-signing-key', await crypto.subtle.verify(
'Ed25519',
await importPublicKey(keys.publicKey),
signature,
payloadBytes
))
const tamperedSignature = signature.slice()
tamperedSignature[0] = (tamperedSignature[0] ?? 0) ^ 1
console.log('Buffer.slice shares memory', signature[0] === tamperedSignature[0])
console.log('signature-now-tampered', signature[0] === tamperedSignature[0])
console.log('tampered-signature-with-signing-key-rejected', await crypto.subtle.verify(
'Ed25519',
await importPublicKey(keys.publicKey),
tamperedSignature,
payloadBytes
))
console.log('original-signature-would-be-tampered', signature[0] === tamperedSignature[0])
tamperedSignature[0] = (tamperedSignature[0] ?? 0) ^ 1
const signatureCpy = Buffer.from(signature)
console.log('Buffer.from copies', signatureCpy[0] !== signature[0])
console.log('remaining-original-is-now-tampered-but-copied-original-is-not', signatureCpy[0] !== tamperedSignature[0])
JSRepository: kentcdodds/kody
Length of output: 1017
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe using Node ES modules and only available crypto APIs.
node --input-type=module - <<'JS'
import { generateKeyPairSync } from 'node:crypto'
const keys = generateKeyPairSync('Ed25519')
const wrong = generateKeyPairSync('Ed25519')
async function importPublicKey(key) {
const der = key.export({ format: 'der', type: 'spki' })
return crypto.subtle.importKey('spki', der, 'Ed-25519', false, ['verify'])
}
// Use a short UTF8 byte payload that keeps key material out of stdout.
const payloadBytes = new TextEncoder().encode('\xff\x00\x01')
const signer = await crypto.subtle.importKey(
'pkcs8',
Buffer.from(keys.privateKey.export({ format: 'der', type: 'pkcs8' })),
'Ed25519',
false,
['sign']
)
const signatureArray = await crypto.subtle.sign('Ed25519', signer, payloadBytes)
const signature = Buffer.from(signatureArray)
const originalSigningKeyValid = await crypto.subtle.verify(
'Ed25519',
await importPublicKey(keys.publicKey),
signature,
payloadBytes
)
const tamperedSignature = signature.slice()
tamperedSignature[0] = (tamperedSignature[0] ?? 0) ^ 1
const signatureSharesMemoryWithTampered = signature[0] === tamperedSignature[0]
const tamperedSigningKeyValid = await crypto.subtle.verify(
'Ed25519',
await importPublicKey(keys.publicKey),
tamperedSignature,
payloadBytes
)
const recoveredSignature = Buffer.from(signature)
const recoveredSignatureSharingCheck = recoveredSignature[0] !== tamperedSignature[0]
const recoveredSigningKeyValid = await crypto.subtle.verify(
'Ed25519',
await importPublicKey(keys.publicKey),
recoveredSignature,
payloadBytes
)
const wrongSigningKeyValid = await crypto.subtle.verify(
'Ed25519',
await importPublicKey(wrong.publicKey),
recoveredSignature,
payloadBytes
)
console.log(JSON.stringify({
originalSigningKeyValid,
signatureSharesMemoryWithTampered,
tamperedSigningKeyValid,
recoveredSignatureSharingCheck,
recoveredSigningKeyValid,
wrongSigningKeyValid,
}, null, 2))
JSRepository: kentcdodds/kody
Length of output: 983
Copy the signature buffer before tampering.
Buffer.slice() returns a view of the same memory, so mutating tamperedSignature[0] also changes signature[0]. The wrong-key assertion then validates a corrupted Ed25519 signature instead of the original valid one; keep the assertions independent by copying the buffer.
💚 Proposed fix
- const tamperedSignature = signature.slice()
+ const tamperedSignature = Buffer.from(signature)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tamperedSignature = signature.slice() | |
| tamperedSignature[0] = (tamperedSignature[0] ?? 0) ^ 1 | |
| assert.equal( | |
| await crypto.subtle.verify( | |
| 'Ed25519', | |
| await importPublicKey(signingKeys.publicKey), | |
| tamperedSignature, | |
| payloadBytes, | |
| ), | |
| false, | |
| ) | |
| assert.equal( | |
| await crypto.subtle.verify( | |
| 'Ed25519', | |
| await importPublicKey(wrongKeys.publicKey), | |
| signature, | |
| payloadBytes, | |
| ), | |
| false, | |
| ) | |
| const tamperedSignature = Buffer.from(signature) | |
| tamperedSignature[0] = (tamperedSignature[0] ?? 0) ^ 1 | |
| assert.equal( | |
| await crypto.subtle.verify( | |
| 'Ed25519', | |
| await importPublicKey(signingKeys.publicKey), | |
| tamperedSignature, | |
| payloadBytes, | |
| ), | |
| false, | |
| ) | |
| assert.equal( | |
| await crypto.subtle.verify( | |
| 'Ed25519', | |
| await importPublicKey(wrongKeys.publicKey), | |
| signature, | |
| payloadBytes, | |
| ), | |
| false, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/backup-control-plane/manifest-signing.node.test.ts` around lines 81
- 100, Update the tamperedSignature setup in the signature verification test to
create an independent copy of signature before mutating its first byte. Preserve
the original signature for the subsequent wrongKeys.publicKey verification
assertion.
cursor
Bot
changed the title
2 participants
This draft was created from the stale pre-merge branch by the branch-implicit PR tool and is not the clean follow-up requested after #884. Do not review or merge it. The correct follow-up is created from
cursor/dr-provenance-follow-up, based on currentmain.Summary by CodeRabbit