Skip to content

fix(yucca sdk): skip snapshot prune if warnings are present - #3

Open
insertish wants to merge 4 commits into
fix/retention-policyfrom
fix/skip-pruning-after-warnings
Open

insertish wants to merge 4 commits into
fix/retention-policyfrom
fix/skip-pruning-after-warnings

Conversation

@futo-kritika

futo-kritika Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Re-runViewConfidence Score: 4/5

[Medium risk] When the unreadable-source error is caught, summary stays undefined. The lines after the try/catch, including the "Finished backup to primary backend" log and anything later that uses the summary, may dereference it, and the review does not say it checked this. The diff also adds no import of ResticBackupCommandCouldNotReadSourceDataError to the test file. The risk rating rests on this being ordinary backup and retention logic that a revert undoes, although skipping the prune could let …

Approved

Summary

Skips backup retention pruning when warnings are present.

RepositoryService.createBackup now collects unreadable-source errors alongside integration-hook warnings and runs automatic forget/prune only when no warnings remain. Warning-bearing backups still finish through TaskWarningsError, preserving the warning task status. The integration test exercises this path with a mocked unreadable-source error.

What's good

  • Keeps unexpected backup errors fatal while collecting unreadable-source errors as warnings.
  • Adds a regression test asserting warning completion without invoking retention deletion.
Review context · 0/6 commands
  • Commands run: none

Reviews (2) · Last reviewed commit: "Merge branch 'fix/retention-policy' into..." · openai/gpt-6.1-sol · $0.12

@futo-kritika futo-kritika Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kritika: confidence 4/5 with medium risk at c1fb9f3.

@futo-kritika futo-kritika Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kritika: confidence 4/5 with medium risk at f616b93.

This branch has not been deployed

No deployments
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