fix(database): pull migrations from an explicit branch - #8398
Conversation
`netlify db migrations pull` left the branch undefined unless --branch was passed, so neither request carried a branch query parameter. The list endpoint defaults to production server-side and returned the full set, but the detail endpoint fell back to the site's published deploy, whose manifest no longer lists a migration that was applied and later deleted from the repo. The command therefore failed with a 404 on exactly the files it exists to restore, while reporting "production" as the source it had pulled from. Resolve the branch to production when neither --branch nor NETLIFY_DB_BRANCH is set, and always send it. The fetch helpers now take a required branch so the parameter cannot silently go missing again. This also makes the reported source match the branch actually requested.
commit: |
📝 WalkthroughSummary by CodeRabbit
WalkthroughMigration pulling now resolves an explicit branch from the request, environment configuration, or Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/database/db-migration-pull.ts (1)
119-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove explanatory comments from both changed TypeScript files.
src/commands/database/db-migration-pull.ts#L119-L122: remove the branch-resolution explanation.tests/unit/commands/database/db-migration-pull.test.ts#L182-L184: remove the endpoint-fallback explanation.As per coding guidelines:
**/*.{ts,tsx}files must not contain comments that describe what the code does.🤖 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 `@src/commands/database/db-migration-pull.ts` around lines 119 - 122, Remove the explanatory branch-resolution comment near the migration pull logic in src/commands/database/db-migration-pull.ts (lines 119-122) and the endpoint-fallback comment in tests/unit/commands/database/db-migration-pull.test.ts (lines 182-184); leave the surrounding implementation and test behavior unchanged.Source: Coding guidelines
🤖 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 `@src/commands/database/db-migration-pull.ts`:
- Line 123: Update the branch selection near resolveBranch to obtain
NETLIFY_DB_BRANCH through the shared configuration resolver, using
command.netlify.cachedConfig.env, before falling back to PRODUCTION_BRANCH.
Remove the explanatory comments above this logic and preserve the explicit
options.branch precedence.
---
Nitpick comments:
In `@src/commands/database/db-migration-pull.ts`:
- Around line 119-122: Remove the explanatory branch-resolution comment near the
migration pull logic in src/commands/database/db-migration-pull.ts (lines
119-122) and the endpoint-fallback comment in
tests/unit/commands/database/db-migration-pull.test.ts (lines 182-184); leave
the surrounding implementation and test behavior unchanged.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fdf6e25-d71b-4221-bbd5-fd2f74de1960
📒 Files selected for processing (2)
src/commands/database/db-migration-pull.tstests/unit/commands/database/db-migration-pull.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
| // endpoint fall back to the published deploy, which 404s for a migration that | ||
| // was applied and later deleted from the repo — the files this command exists | ||
| // to restore. It also made the request disagree with the branch reported below. | ||
| const branch = (await resolveBranch(options.branch)) ?? process.env.NETLIFY_DB_BRANCH ?? PRODUCTION_BRANCH |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 'NETLIFY_DB_BRANCH|dotenv|loadEnv|site settings|addon|build-time' src tests
rg -n -C 4 'getApiContext|BaseCommand' src/commands src/utilsRepository: netlify/cli
Length of output: 50368
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- migration command ---'
sed -n '1,230p' src/commands/database/db-migration-pull.ts
printf '%s\n' '--- command registration and lifecycle ---'
rg -n -C 8 'migrationPull|db-migration-pull|loadDotEnvFiles|getDotEnvVariables|cachedConfig\.env|process\.env' \
src/commands/database src/commands/base-command.ts src/utils src/lib \
-g '*.ts' -g '*.js' | head -n 500
printf '%s\n' '--- environment resolver exports and callers ---'
rg -n -C 8 'export .*get.*Env|function get.*Env|getDotEnvVariables|EnvironmentVariables|filterEnvBySource|load.*Env' \
src/utils src/commands -g '*.ts' | head -n 500Repository: netlify/cli
Length of output: 50367
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- BaseCommand initialization ---'
sed -n '560,880p' src/commands/base-command.ts
printf '%s\n' '--- BaseCommand environment-related members ---'
rg -n -C 6 'cachedConfig|config\.env|env\b|injectEnvVariables|getEnvelopeEnv|getDotEnvVariables|resolveConfig' \
src/commands/base-command.ts src/commands/database/database.ts src/commands/database \
-g '*.ts'
printf '%s\n' '--- database command tests for branch sources ---'
sed -n '400,485p' tests/unit/commands/database/db-migration-pull.test.tsRepository: netlify/cli
Length of output: 35687
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
base = Path("src/commands/base-command.ts").read_text()
migration = Path("src/commands/database/db-migration-pull.ts").read_text()
init = base[base.index("private async init"):base.index("/** Find and resolve the Netlify configuration")]
print("migration reads process.env.NETLIFY_DB_BRANCH:",
"process.env.NETLIFY_DB_BRANCH" in migration)
print("init injects resolved env into process.env:",
bool(re.search(r"injectEnvVariables|getDotEnvVariables|process\\.env\\[", init)))
print("init stores cachedConfig.env:",
bool(re.search(r"cachedConfig\\?\\.env|cachedConfig:\\s*\\{.*?env", init, re.S)))
for path in Path("src").rglob("*.ts"):
text = path.read_text()
if "injectEnvVariables(" in text:
print("injectEnvVariables caller:", path)
PY
printf '%s\n' '--- all environment injection callers ---'
rg -n 'injectEnvVariables|getDotEnvVariables|loadDotEnvFiles' src --glob '*.ts'Repository: netlify/cli
Length of output: 1009
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
base = Path("src/commands/base-command.ts").read_text()
migration = Path("src/commands/database/db-migration-pull.ts").read_text()
start = base.index("private async init")
end = base.index("/** Find and resolve the Netlify configuration")
init = base[start:end]
print("migration reads process.env.NETLIFY_DB_BRANCH:",
"process.env.NETLIFY_DB_BRANCH" in migration)
print("init calls injectEnvVariables:",
"injectEnvVariables" in init)
print("init calls getDotEnvVariables:",
"getDotEnvVariables" in init)
print("init assigns process.env[...] =:",
"process.env[" in init)
print("init reads cachedConfig.env:",
"cachedConfig?.env" in init)
print("init stores cachedConfig:",
"cachedConfig: {" in init)
for path in Path("src").rglob("*.ts"):
text = path.read_text()
if "injectEnvVariables(" in text:
print("injectEnvVariables caller:", path)
PY
printf '%s\n' '--- all environment injection callers ---'
rg -n 'injectEnvVariables|getDotEnvVariables|loadDotEnvFiles' src --glob '*.ts'Repository: netlify/cli
Length of output: 2845
Resolve NETLIFY_DB_BRANCH through the shared configuration.
BaseCommand stores resolved environment variables in command.netlify.cachedConfig.env but does not inject them into process.env for this command. Line 123 therefore ignores .env variants, site settings, addon variables, and build-time configuration when process.env.NETLIFY_DB_BRANCH is unset. Use the shared resolver before falling back to production. Remove the explanatory comments above this line.
🤖 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 `@src/commands/database/db-migration-pull.ts` at line 123, Update the branch
selection near resolveBranch to obtain NETLIFY_DB_BRANCH through the shared
configuration resolver, using command.netlify.cachedConfig.env, before falling
back to PRODUCTION_BRANCH. Remove the explanatory comments above this logic and
preserve the explicit options.branch precedence.
Source: Coding guidelines
Summary
netlify db migrations pullleft the branch undefined unless --branch was passed, so neither request carried a branch query parameter. The list endpoint defaults to production server-side and returned the full set, but the detail endpoint fell back to the site's published deploy, whose manifest no longer lists a migration that was applied and later deleted from the repo. The command therefore failed with a 404 on exactly the files it exists to restore, while reporting "production" as the source it had pulled from.Resolve the branch to production when neither --branch nor NETLIFY_DB_BRANCH is set, and always send it. The fetch helpers now take a required branch so the parameter cannot silently go missing again. This also makes the reported source match the branch actually requested.
For us to review and ship your PR efficiently, please perform the following steps:
can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)