Skip to content

fix(database): pull migrations from an explicit branch - #8398

Merged
paulo merged 1 commit into
mainfrom
pa/run-3224-migration-pull-branch-default
Aug 12, 2026
Merged

fix(database): pull migrations from an explicit branch#8398
paulo merged 1 commit into
mainfrom
pa/run-3224-migration-pull-branch-default

Conversation

@paulo

@paulo paulo commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

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.


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we
    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.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

`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.
@github-actions

Copy link
Copy Markdown

📊 Benchmark results

Comparing with 942ca8f

  • Dependency count: 1,165 (no change)
  • Package size: 455 MB ⬆️ 0.00% increase vs. 942ca8f
  • Number of ts-expect-error directives: 346 (no change)

@pkg-pr-new

pkg-pr-new Bot commented Aug 12, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/netlify-cli@8398

commit: c13f585

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Database migration pulls now consistently target an explicit branch.
    • When no branch is specified, migrations default to the production branch.
    • Branch information is now displayed consistently in status messages, confirmation prompts, and JSON output.
    • Migration list and migration content requests now use the same resolved branch.

Walkthrough

Migration pulling now resolves an explicit branch from the request, environment configuration, or production. List and content requests always include that branch. Empty-result messages, overwrite prompts, JSON output, and success messages report the resolved branch. Unit tests verify production branch parameters and logging when no branch is specified.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: amun-sihra

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: pulling migrations from an explicit branch.
Description check ✅ Passed The description explains the branch-resolution change, its cause, its effect on requests, and the added safeguards.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pa/run-3224-migration-pull-branch-default

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@paulo
paulo marked this pull request as ready for review August 12, 2026 13:49
@paulo
paulo requested a review from a team as a code owner August 12, 2026 13:49
@paulo
paulo requested a review from eduardoboucas August 12, 2026 13:49

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/commands/database/db-migration-pull.ts (1)

119-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove 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

📥 Commits

Reviewing files that changed from the base of the PR and between 942ca8f and c13f585.

📒 Files selected for processing (2)
  • src/commands/database/db-migration-pull.ts
  • tests/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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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/utils

Repository: 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 500

Repository: 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.ts

Repository: 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

@paulo
paulo merged commit 6d5b690 into main Aug 12, 2026
38 checks passed
@paulo
paulo deleted the pa/run-3224-migration-pull-branch-default branch August 12, 2026 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants