Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions src/commands/database/db-migration-pull.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,9 @@ const getApiContext = (command: BaseCommand): ApiContext => {
}
}

const fetchMigrations = async (ctx: ApiContext, branch: string | undefined): Promise<MigrationListItem[]> => {
const fetchMigrations = async (ctx: ApiContext, branch: string): Promise<MigrationListItem[]> => {
const url = new URL(`${ctx.basePath}/sites/${encodeURIComponent(ctx.siteId)}/database/migrations`)
if (branch) {
url.searchParams.set('branch', branch)
}
url.searchParams.set('branch', branch)

const response = await fetch(url, {
headers: { Authorization: `Bearer ${ctx.token}` },
Expand All @@ -96,13 +94,11 @@ const fetchMigrations = async (ctx: ApiContext, branch: string | undefined): Pro
return data.migrations
}

const fetchMigrationContent = async (ctx: ApiContext, name: string, branch: string | undefined): Promise<string> => {
const fetchMigrationContent = async (ctx: ApiContext, name: string, branch: string): Promise<string> => {
const url = new URL(
`${ctx.basePath}/sites/${encodeURIComponent(ctx.siteId)}/database/migrations/${encodeURIComponent(name)}`,
)
if (branch) {
url.searchParams.set('branch', branch)
}
url.searchParams.set('branch', branch)

const response = await fetch(url, {
headers: { Authorization: `Bearer ${ctx.token}` },
Expand All @@ -120,16 +116,19 @@ const fetchMigrationContent = async (ctx: ApiContext, name: string, branch: stri
export const migrationPull = async (options: MigrationPullOptions, command: BaseCommand) => {
const { force, json } = options

const branch = (await resolveBranch(options.branch)) ?? process.env.NETLIFY_DB_BRANCH
const source = branch ?? PRODUCTION_BRANCH
// Always resolve to an explicit branch. Leaving it undefined makes the detail
// 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

const ctx = getApiContext(command)
const migrations = await fetchMigrations(ctx, branch)

if (migrations.length === 0) {
if (json) {
logJson({ migrations_pulled: 0, branch: source })
logJson({ migrations_pulled: 0, branch })
} else {
log(`No migrations found for ${source}.`)
log(`No migrations found for ${branch}.`)
}
return
}
Expand All @@ -155,7 +154,7 @@ export const migrationPull = async (options: MigrationPullOptions, command: Base
name: 'confirmed',
message: `This will overwrite all local migrations in ${migrationsDirectory} with ${String(
migrations.length,
)} migration${migrations.length === 1 ? '' : 's'} from ${source}. Continue?`,
)} migration${migrations.length === 1 ? '' : 's'} from ${branch}. Continue?`,
default: false,
},
])
Expand All @@ -178,11 +177,11 @@ export const migrationPull = async (options: MigrationPullOptions, command: Base
if (json) {
logJson({
migrations_pulled: migrations.length,
branch: source,
branch,
migrations: migrations.map((m) => m.name),
})
} else {
log(`Pulled ${String(migrations.length)} migration${migrations.length === 1 ? '' : 's'} from ${source}:`)
log(`Pulled ${String(migrations.length)} migration${migrations.length === 1 ? '' : 's'} from ${branch}:`)
for (const migration of migrations) {
log(` - ${migration.name}`)
}
Expand Down
26 changes: 21 additions & 5 deletions tests/unit/commands/database/db-migration-pull.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ describe('migrationPull', () => {
await migrationPull({}, createMockCommand())

const calledUrl = mockFetch.mock.calls[0][0] as URL
expect(calledUrl.toString()).toBe('https://api.netlify.com/api/v1/sites/site-123/database/migrations')
expect(calledUrl.toString()).toBe(
'https://api.netlify.com/api/v1/sites/site-123/database/migrations?branch=production',
)
expect(mockFetch.mock.calls[0][1]).toEqual({ headers: { Authorization: 'Bearer test-token' } })
})

Expand All @@ -168,14 +170,28 @@ describe('migrationPull', () => {

expect(detailCalls).toHaveLength(2)
expect(detailCalls.map((u) => u.toString()).sort()).toEqual([
'https://api.netlify.com/api/v1/sites/site-123/database/migrations/0001_create-users',
'https://api.netlify.com/api/v1/sites/site-123/database/migrations/0002_add-posts',
'https://api.netlify.com/api/v1/sites/site-123/database/migrations/0001_create-users?branch=production',
'https://api.netlify.com/api/v1/sites/site-123/database/migrations/0002_add-posts?branch=production',
])
for (const call of mockFetch.mock.calls) {
expect(call[1]).toEqual({ headers: { Authorization: 'Bearer test-token' } })
}
})

test('requests the production branch it reports pulling from when none is given', async () => {
// Sending no branch makes the detail endpoint resolve against the published
// deploy, which 404s for a migration that was applied and later deleted from
// the repo — exactly the files this command exists to restore.
mockFetchResponse(sampleMigrations)

await migrationPull({ force: true }, createMockCommand())

const branches = mockFetch.mock.calls.map((call) => (call[0] as URL).searchParams.get('branch'))
expect(branches.length).toBeGreaterThan(0)
expect([...new Set(branches)]).toEqual(['production'])
expect(logMessages.join('\n')).toContain('from production')
})

test('forwards branch to both list and detail endpoints', async () => {
mockFetchResponse(sampleMigrations)

Expand Down Expand Up @@ -393,13 +409,13 @@ describe('migrationPull', () => {
expect(calledUrl.searchParams.get('branch')).toBe('feature/my-branch')
})

test('does not send branch query parameter when --branch is not used', async () => {
test('sends the production branch explicitly when --branch is not used', async () => {
mockFetchResponse(sampleMigrations)

await migrationPull({ force: true }, createMockCommand())

const calledUrl = mockFetch.mock.calls[0][0] as URL
expect(calledUrl.searchParams.has('branch')).toBe(false)
expect(calledUrl.searchParams.get('branch')).toBe('production')
})

test('uses branch name in log messages', async () => {
Expand Down
Loading