Skip to content

fix: sort rollback version files in descending timestamp order (LIFO)#1509

Open
swattamw2024 wants to merge 1 commit into
krkn-chaos:mainfrom
swattamw2024:fix/sort-rollback-version-files
Open

fix: sort rollback version files in descending timestamp order (LIFO)#1509
swattamw2024 wants to merge 1 commit into
krkn-chaos:mainfrom
swattamw2024:fix/sort-rollback-version-files

Conversation

@swattamw2024

Copy link
Copy Markdown

Fixes #1487

Type of change

  • Refactor
  • New feature
  • Bug fix
  • Optimization

Description

Rollback version files and context directories were being processed in arbitrary (filesystem) order. This caused rollbacks to execute in non-deterministic order instead of LIFO (newest-first), which is required for correct rollback semantics.

This fix sorts:

  1. Rollback context directories by their embedded nanosecond timestamp in descending order
  2. Version files by their embedded nanosecond timestamp in descending order

Related Tickets & Documents

  • Related Issue #: 1487
  • Closes #: 1487

Documentation

  • Is documentation needed for this update?

Checklist before requesting a review

  • Ensure the changes and proposed solution have been discussed in the relevant issue and have received acknowledgment from the community or maintainers.
  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough unit tests with above 80% coverage

REQUIRED:
Description of combination of tests performed and output of run

python -m unittest tests.test_rollback -v

Two new test cases added:
- test_search_returns_files_in_descending_timestamp_order — verifies LIFO ordering within a single context directory
- test_search_returns_files_across_contexts_in_descending_order — verifies correct ordering across multiple context directories

@github-actions

Copy link
Copy Markdown

Thanks for your contribution! However, this PR is linked to issue #1487, which is not assigned to you. Please refer to our contribution guidelines. PRs should only be opened by contributors who are assigned to the linked issue.

@augmentcode

augmentcode Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR Risk Analyzer Agent🛡️

👀 Human Input Needed
A pair-review briefing is ready for you.

→ Pair Review Briefing

@augmentcode augmentcode 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.

Deep Code Review Agent🐛

Review completed with 1 suggestion.

Fix in Cosmos

Comment thread krkn/rollback/config.py
logger.warning(
f"File {file} does not match expected pattern of <{scenario_type or '*'}>_<timestamp>_<hash_suffix>.py"
)
version_files.sort(

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.

Deep Code Review Agent🐛

The intra-context LIFO this sort promises won't hold in real usage because Version.timestamp and hash_suffix at config.py:266-267 are @dataclass defaults evaluated once at class-definition time — so every Version.new_version(...) call in the same process produces an identical <scenario>_<timestamp>_<hash>.py, making the sort key constant within a context and collapsing this into a stable no-op over os.listdir order. The new test_search_returns_files_in_descending_timestamp_order only passes because it hand-crafts filenames with distinct timestamps rather than routing through Version.

Severity: medium


🤖 Was this useful? React with 👍 or 👎

@github-actions

Copy link
Copy Markdown

Hi @swattamw2024! 👋 This PR has merge conflicts with main and needs to be rebased before it can be merged.

To rebase your branch:

git fetch upstream
git rebase upstream/main
# resolve any conflicts, then:
git add .
git rebase --continue
git push --force-with-lease

Tip: --force-with-lease is safer than --force — it will fail if someone else has pushed to your branch since your last fetch.

For more details, see the Rebase with upstream guide.

The needs-rebase label and this comment will be removed automatically once conflicts are resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Rollback files execute in non-deterministic order due to unsorted os.listdir()

1 participant