Copy-DbaDatabase - Fix empty -NewName destination sweep and piped multi-database failures

[potatoqualitee]

Fixes #10512

What was going wrong

Reproduced in a lab (sql2017 -> sql2019, UNC share) against the reported 2.8.3 symptoms. Four distinct defects compound into the reported behavior:

1. A bound-but-empty -NewName silently sweeps the destination. [string]$NewName binds $null/"" without validation, $destinationDbName ends up empty, and every downstream -Database "" filter means "all databases". Since 2.8.3 replaced the post-restore ALTER DATABASE with Set-DbaDbOwner, that empty filter rewrites the owner of every updateable database on the destination instance - the reported owner warnings are just the databases it could not touch. The only guard ($dbCount -gt 1) never fires for piped input because process{} sees one database per record.

2. One failed restore silently discards every remaining piped database. The restore-failure path called Stop-Function without -Continue, which sets the module's persistent interrupt flag; all later pipeline records are dropped with no output at all - the "databases just vanish" flavor in the report.

3. A missing single backup file passes verification. Test-DbaPath returns a bare [bool] for a single scalar path on a single instance, but Test-DbaBackupInformation always assumes objects: $path.FileExists -eq $false on a bool is never true, so a missing single-file backup adds no verification error and the engine's OS error 2 becomes the first (confusing) signal.

4. Path-check batch failures are reported as "file not found". When the xp_fileexist batch throws, Test-DbaPath fabricated FileExists = $false for every path and demoted the real exception to Verbose - making transient access failures look like missing backups.

The remaining trigger - backups intermittently "not found" although they exist - reproduced as the destination host's SMB client FileNotFoundCache (default 5 s): a single early miss pins "not found" for the restore that follows (control 0/30 failures, cache primed 30/30, cache disabled 0/15).

The fixes

• Reject a bound-but-empty -NewName up front with a clear message (it is what detonates the owner sweep). An empty -Prefix remains a harmless no-op and is deliberately not rejected.
• Atomic pipeline rejection for -NewName: with piped databases the total count is unknowable until the pipeline ends - by which time the first database would already have been copied under the new name. The command now rejects -NewName with pipeline input in process{} before any work happens ($MyInvocation.ExpectingInput), directing the caller to -Database + -NewName for a single-database rename. A parameter-bound -InputObject array still goes through the original count guard because its full count is known up front.
• Containment invariant: if a destination database name ever resolves to empty, that database emits a Failed migration object and is skipped before any destination-wide command can see an empty filter.
• Restore failures now emit the Failed migration object and use -Continue, so later piped databases still process.
• Bounded visibility retry across the SMB negative-cache window: after a fresh backup, the destination re-probes the backup file(s) for up to 8 seconds (the negative cache expires 5 seconds after creation), warning if they never become visible. This closes the intermittent OS-error-2 restores without touching machine-wide SMB configuration; Set-SmbClientConfiguration -FileNotFoundCacheLifetime 0 on the destination host remains a workable environment-level mitigation.
• Test-DbaBackupInformation handles the scalar-bool contract of Test-DbaPath for single-file backups.
• Test-DbaPath surfaces batch execution failures as a warning stating existence could not be determined, instead of silently reporting not-found.

Verification

• Lab repro matrix before/after: empty -NewName now stops loudly with zero copies and untouched owners; piped multi-database copies complete; piped -NewName is rejected with zero databases created on the destination; a missing backup mid-pipeline yields exactly one Failed row and the next database still copies; parameter-mode single-database rename still works.
• Tests: unit contexts for the empty-name rejection, the scalar-path verification failure, and the indeterminate path-check warning; integration context covering parameter-array and piped multi-database copies (asserting per-database DestinationDatabase and an unrelated destination owner preserved - the sentinel owner is a dedicated login, since the sweep sets owners to the source owner and sa-to-sa would go undetected), atomic piped--NewName rejection, and failed-restore continuation. 12 unit + 5 integration tests green in the lab.

🤖 Generated with Claude Code

[potatoqualitee]

Both reviews addressed in 03cace2. Point by point:

1. Piped-NewName guard fired only after the first database was already copied (both reviews, blocking). Agreed - the cumulative counter was not atomic, and with -WithReplace the partial copy could overwrite a destination database before the rejection fired. The counter machinery is gone. The command now rejects -NewName + pipeline input at the top of process{} before any work happens, detected via $MyInvocation.ExpectingInput: in pipeline mode the total count is unknowable until the pipeline ends, so rejection (not buffering) is the only shape that preserves both atomicity and the streaming design. The message directs callers to -Database + -NewName for a single-database rename. A parameter-bound -InputObject array falls through to the original $dbCount -gt 1 guard because its full count is known up front. The test is renamed to "Rejects NewName with piped databases before any copy happens" and now asserts $results | Should -BeNullOrEmpty plus zero databases created on the destination (neither the rename target nor either source name). Verified in the lab: the run completes in ~100 ms with the destination untouched.

2. Empty -Prefix rejection was an unrelated backward-incompatible change (both reviews). Removed, along with its unit test. An empty prefix is a harmless no-op ($destinationDbName stays the source name), and rejecting it would break existing Prefix = "" splats. Empty -NewName is still rejected because it is the value that detonates the owner sweep.

3. Containment invariant emitted no Failed result (review 1 point 3, review 2 "smaller"). Fixed - $copyDatabaseStatus is now constructed before the invariant, which emits a Failed MigrationObject (Notes = "Destination database name resolved to an empty string") before Stop-Function -Continue, so a skipped database is visible in the output instead of silently vanishing.

4. Scope of "Fixes #10512" vs the SMB trigger (review 1 point 4). Took the bounded-retry option the review favored: after a fresh backup (skipped for -UseLastBackup and URL paths), the destination re-probes the backup file(s) for up to 8 seconds at 1-second intervals. The negative-cache entry expires 5 seconds after creation regardless of intervening probes, so the window is guaranteed to be crossed; if the files still are not visible the command warns and lets the restore surface the underlying error. The retry handles both return shapes of Test-DbaPath (bare bool for a single scalar path, objects otherwise). "Fixes #10512" stays.

5. Failed-restore continuation test (review 1 "missing regression test", review 2 point 3). Added: "Continues to the next piped database when one restore fails". It backs up both databases to the share, deletes the first backup file (history intact, so -UseLastBackup deterministically hands the restore a missing file - no timing race), pipes both databases, and asserts exactly one result row per database, first Failed, second Successful and present on the destination. This is also the reported no-NewName command shape: on 2.8.3 this scenario loses the second database with no output at all.

6. Sentinel-owner false negative (review 2 point 4). Fixed - the sentinel database owner is now a dedicated dbatoolsci_owner* login created for the test. The sweep sets destination owners to the source database owner, so with both at sa a sweep was undetectable; now any sweep flips the sentinel to sa and the assert catches it. The piped-success test also asserts nonblank DestinationDatabase per database, closing review 2's "empty-NewName test only exercises begin validation" gap: the happy-path pipe run now proves the resolved destination name per record.

Lab verification after the rework: 12 unit + 5 integration tests green, plus a live parameter-mode -Database + -NewName rename check (Successful, restored under the new name). The Test-DbaPath scalar-bool fix and the Verbose-to-Warning change both reviews endorsed are unchanged.

🤖 Generated with Claude Code

[potatoqualitee]

Round-2 review feedback addressed in 4212762. Point by point:

Empty/incomplete visibility probe treated as success — fixed. The retry now materializes the Test-DbaPath results and requires one conclusive result for every backup path (count match against the requested list, plus a non-empty list) before anything counts as visible. An indeterminate probe — Test-DbaPath emits nothing at all when its own connection attempt fails via Stop-Function -Continue — now keeps retrying until the negative-cache deadline and then emits the timeout warning.

Retry state-machine coverage — added Keeps retrying when the visibility probe returns no results: a module-scoped mock of Test-DbaPath, filtered to the backup file path so the -SharedPath folder checks still hit the real command, returns nothing; the test asserts the copy waits out the window and emits the negative-cache warning instead of proceeding. This one test exercises both the no-output case and persistent-not-visible-through-deadline (every probe is indeterminate until the deadline). The false→true recovery transition drives the identical loop machinery — not-visible, sleep, re-probe — and both exit conditions (visible, deadline) are covered between this test and the immediately-visible copies elsewhere in the suite, so I stopped at one focused test rather than three.

Continuation test ordering — fixed. The failing database is now piped explicitly first via two separate Get-DbaDatabase calls; the test no longer leans on sys.databases emission order, and a failure on the last record can no longer mask the old discard behavior.

NewName help — updated to state the pipeline-input restriction and the escape hatch: specify the database with -Database and provide -NewName.

Single-database piped rename (the P1 in one review) — kept the atomic rejection, deliberately. The other two reviews endorse it as the right implementation, and buffering pipeline input to allow the one-record case was considered and rejected in the previous round: it would restructure the streaming process{} flow of a very large command inside a regression fix, for a form the docs never promised. The guard fails fast before any destination work with an actionable message, and the help now documents the restriction. If the maintainers want the buffer-one variant, I'm happy to do it as a follow-up PR where the restructuring can be reviewed on its own.

🤖 Generated with Claude Code

[potatoqualitee]

The CI failure on 4212762 was the new mock-based retry test — the module-scoped Mock Test-DbaPath intercepted the internal -SharedPath folder probe, and a Pester 5 filtered mock has no real-command fallback for non-matching calls. Removed the test in e5c56fa per project policy (integration tests only, no mocks). The no-output probe path can't be driven from real instances, so it stays untested; the conclusive-result guard itself remains in the production retry loop and the rest of the suite covers the visibility behavior.

[potatoqualitee]

github actions are broken :(