Skip to content

If a solution exists but we want to remove it it should be deleted from the remote #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ColdHeat merged 1 commit into from
Feb 26, 2026
Merged

If a solution exists but we want to remove it it should be deleted from the remote #200

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion ctfcli/core/challenge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,9 @@ def sync(self, ignore: tuple[str] = ()) -> None:
self._set_next(_next)

if "solution" not in ignore:
# self._delete_existing_solution()
resolved_solution = self._resolve_solution_path()
if not resolved_solution:
self._delete_existing_solution()
Comment on lines +922 to +924
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The solution sync logic has inefficiencies and inconsistencies compared to other fields:

  1. Duplicate path resolution: _resolve_solution_path() is called twice - once here and once inside _create_solution(). This happens in both cases (solution exists or not).

  2. Pattern inconsistency: This doesn't follow the established pattern used for flags (lines 847-850), topics (lines 853-856), tags (lines 859-862), and hints (lines 907-910), which always delete then conditionally create.

Consider restructuring to match the established pattern:

  • Always call _delete_existing_solution()
  • Conditionally call _create_solution() (which already returns early if no solution)

This would eliminate the duplicate path resolution and improve code consistency.

Suggested change
resolved_solution = self._resolve_solution_path()
if not resolved_solution:
self._delete_existing_solution()
self._delete_existing_solution()

Copilot uses AI. Check for mistakes.
self._create_solution()
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The new behavior to delete existing solutions when no local solution is defined lacks test coverage. Given that the TestSyncChallenge class has comprehensive tests for similar delete-on-removal behavior for other fields (flags, topics, tags, hints, files), a test should be added for solutions as well.

Consider adding a test that:

  1. Sets up a challenge with an existing remote solution
  2. Syncs with no local solution defined (or solution file missing)
  3. Verifies that the delete API call is made to remove the remote solution
Suggested change
self._create_solution()
else:
self._create_solution()

Copilot uses AI. Check for mistakes.

make_challenge_visible = False
Expand Down