Restore-DbaDatabase - Fix copy-only point-in-time chains

[potatoqualitee]

Closes #10408

Summary

Fixes point-in-time restore-chain selection when the newest full backup is copy-only. Transaction log backups continue to reference the conventional full backup through DatabaseBackupLSN, so comparing that value with the copy-only full's checkpoint LSN incorrectly rejected the terminal log.

What changed

• Select applicable logs using the chosen restore base's last LSN and recovery fork.
• Select the log containing RestoreTime without relying on DatabaseBackupLSN.
• Avoid appending the terminal log twice when it is already in the selected chain.
• Add synthetic copy-only history tests for before-start, exact-start, in-range, exact-end, between-log, recovery-fork, and default-future cases.
• Add an SQL-backed integration assertion that STOPAT appears on only the final RESTORE LOG statement.

Verification

• Select-DbaBackupInformation.Tests.ps1: 64 passed, 0 failed.
• PSScriptAnalyzer 1.18.2: clean for Select-DbaBackupInformation.ps1.
• PowerShell parser: clean for all three changed files.
• git diff --check origin/development...HEAD: clean.
• The new Restore-DbaDatabase SQL integration assertion requires the repository CI environment.

Review note

Claude Opus/high review completed after merge with VERDICT: CLEAN.

[potatoqualitee]

@claude this is an incredibly sensitive command. Please give it a xhigh effort review

[claude]

Claude finished @potatoqualitee's task in 6m 27s —— View job

---

Claude's xhigh-effort review of Select-DbaBackupInformation.ps1 restore-chain fix

• Gather context (PR diff, changed files, issue Restore-DbaDatabase -RestoreTime gets set on the wrong transaction log backup when using a copy only full backup #10408)
• Deep-read Select-DbaBackupInformation.ps1 logic changes
• Verify LSN/recovery-fork/STOPAT logic against restore-chain semantics
• Review new tests for coverage and correctness
• Check for edge cases / regressions vs prior behavior
• Post final review

Note on environment: the sandbox's Bash tool failed to initialize this run (socat not installed), so I could not execute git diff, Pester, or PSScriptAnalyzer myself. This review is based on reading the full current source/tests and hand-tracing the algorithm against every new test case. Where I couldn't independently execute something, I say so explicitly below.

Core logic — traced by hand, looks correct

I walked public/Select-DbaBackupInformation.ps1:236-278 against all 6 new cases in the New-CopyOnlyBackupHistory fixture (tests/Select-DbaBackupInformation.Tests.ps1:112-134), including the wrong-fork log (id 5) and the between-log/exact-boundary restore times. Every case resolves exactly as asserted:

• $LogBaseLsn/$FirstRecoveryForkID are now derived from the chosen restore base's own LastLsn/FirstRecoveryForkID (whichever of Full/Diff has the higher LastLsn), not from DatabaseBackupLSN — this is the right fix, since log backups' DatabaseBackupLSN continues to reference the last conventional full even when the newest full is copy-only.
• The terminal-log query ($lastLog, lines 268-277) no longer requires DatabaseBackupLSN -eq $Full.CheckpointLSN, instead using LastLSN -ge $LogBaseLsn + fork-ID match — this correctly lets the terminal log resolve even when the base is a copy-only full.
• The -notin $dbHistory.BackupSetID guard correctly prevents re-adding a log that the earlier $FilteredLogs loop already included as the terminal log (verified against the RestoreTime=02:02/02:05 cases where the same BackupSetID would otherwise be appended twice).

I didn't find a logic error in this file. Nice, minimal fix — it doesn't touch the (unrelated, still-correct) Diff-selection LSN check.

Things worth double-checking (not confirmed bugs, but flagged for the "incredibly sensitive" bar)

1. The new Restore-DbaDatabase.Tests.ps1:438-444 STOPAT assertion exercises code this PR does not touch. Invoke-DbaAdvancedRestore.ps1:389-397 sets $restore.ToPointInTime per backup step based on $backup.RestoreTime — and Select-DbaBackupInformation stamps the same RestoreTime value onto every history object (full, diff, every log), not just the terminal one. I could not run this integration test (no Bash/SQL Server available here) to confirm SMO's Restore.Script() only renders STOPAT on the final/NoRecovery=$false step rather than on every RESTORE LOG statement in the chain. Since this is the one assertion in the PR that needs the real CI environment anyway (per the PR description), please make sure it's actually green in CI before merging — if it already passes, this note can be ignored.

2. Minor style nit: tests/Select-DbaBackupInformation.Tests.ps1:112 uses It "..." -ForEach @(...), which tests/CLAUDE.md explicitly forbids ("Never use the -ForEach parameter on any test blocks"). A few other test files in the repo already do this too, so it's not a new precedent, but worth conforming to the documented standard given how sensitive this command is — consider unrolling into discrete It blocks or per-case Contexts.

3. Behavioral note, not a bug: $FirstRecoveryForkID is always taken from $Full, even when a Diff is the actual restore base (line 239). In practice a differential always shares its full's recovery fork, so this is safe, but calling it out since fork-ID correctness is exactly what this PR is hardening.

Verdict

The core copy-only LSN/fork-ID fix is sound and the new unit tests genuinely exercise the reported bug (I confirmed by hand-simulating the algorithm, not just reading the assertions). The only open item is confirming the new SQL-backed STOPAT integration assertion is actually green in the full CI run, since it touches restore-script-generation code this PR doesn't modify. The -ForEach usage is a pre-existing repo pattern elsewhere, so a nit rather than a blocker.