Skip to content

fix(cli): reject init --force at the curriculum source - #147

Open
Azaucifer wants to merge 1 commit into
abhiksark:mainfrom
Azaucifer:fix/reject-init-source
Open

Azaucifer wants to merge 1 commit into
abhiksark:mainfrom
Azaucifer:fix/reject-init-source

Conversation

@Azaucifer

@Azaucifer Azaucifer commented Sep 16, 2026

Copy link
Copy Markdown

Summary

Fixes #71

init --path <curriculum-source> --force was crashing with shutil.SameFileError because init_workspace() started copying before checking whether the destination resolved to the curriculum source. update_workspace() already had the equivalent guard; this adds the same guard to init_workspace().

Changes

  • pythonlings/core/curriculum.py: reject forced init when path == source_root() before any copy or removal, raising WorkspaceError with a concise message.
  • tests/unit/test_curriculum.py: direct init_workspace() regression test for the same-path case.
  • tests/integration/test_cli_workspace.py: CLI-level regression test asserting non-zero exit and clean pythonlings: error output, plus that the source checkout is unchanged.

Acceptance checklist

  • init --path <curriculum-source> --force exits non-zero without a traceback.
  • The source checkout is unchanged after the rejected command (verified via tree hash comparison).
  • The error identifies that the curriculum source cannot be initialized in place.
  • Forced initialization of a different existing workspace retains its current behavior.
  • First initialization and repeated non-forced initialization retain their current behavior.
  • Regression coverage exercises the resolved same-path case.

Tests

python -m pytest tests/unit/test_curriculum.py tests/integration/test_cli_workspace.py -q
# 25 passed

python -m pytest -q
# all passing

python -m pythonlings --root tests/fixtures/passing_curriculum verify
# ✓ passing1
# ✓ passing2

Same-path repro (disposable copy):

python -m pythonlings init --path . --force
# pythonlings: cannot initialize the curriculum source in place at <path>
# exit code 1, no traceback
# tree hash unchanged before/after

Checklist

  • Updated docs when behavior changed (no docs change needed)
  • Added or updated tests
  • Verified python -m pytest -q

Summary by CodeRabbit

  • Bug Fixes
    • Prevented curriculum initialization in the curriculum’s source directory.
    • The init --force command now exits with an error and preserves existing files when used on the source directory.
  • Tests
    • Added coverage confirming the error response and ensuring source directory contents remain unchanged.

- Add same-path guard in init_workspace() before any copy
- Reuse the existing WorkspaceError CLI error path
- Add unit test for direct init_workspace() rejection
- Add integration test for CLI init --force rejection
- Source checkout is left unchanged after rejection

Closes abhiksark#71
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3de2b3db-1426-4020-b9ea-77c26b6a92fd

📥 Commits

Reviewing files that changed from the base of the PR and between b4d40bf and aa02274.

📒 Files selected for processing (3)
  • pythonlings/core/curriculum.py
  • tests/integration/test_cli_workspace.py
  • tests/unit/test_curriculum.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
Keep Textual imports out of `pythonlings/core/` and one-shot CLI command import paths.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • pythonlings/core/curriculum.py
Maintain Python 3.9 compatibility.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • tests/integration/test_cli_workspace.py
  • pythonlings/core/curriculum.py
  • tests/unit/test_curriculum.py
Preserve learner-edited exercises during workspace updates.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • tests/integration/test_cli_workspace.py
  • pythonlings/core/curriculum.py
  • tests/unit/test_curriculum.py
🔇 Additional comments (3)
pythonlings/core/curriculum.py (1)

95-99: LGTM!

tests/unit/test_curriculum.py (1)

120-142: LGTM!

tests/integration/test_cli_workspace.py (1)

284-299: LGTM!


📝 Walkthrough

Walkthrough

init_workspace now rejects initialization when the resolved target equals the curriculum source. Unit and integration tests verify the error, exit code, and preservation of source files.

Changes

Curriculum source initialization guard

Layer / File(s) Summary
Source guard and validation
pythonlings/core/curriculum.py, tests/unit/test_curriculum.py, tests/integration/test_cli_workspace.py
init_workspace checks the resolved source directory before copying and raises WorkspaceError. Tests verify the error, non-zero CLI exit, and unchanged source files.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: abhiksark

Merge Risk: ⚪ Minimal · up to aa022

The change implements the intended source-directory protection and preserves the established CLI error behavior without an identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting forced initialization at the curriculum source.
Description check ✅ Passed The description includes the change summary, implementation details, linked issue, tests, acceptance criteria, and checklist. It omits the template's Screenshots section, but this is non-critical for …
Linked Issues check ✅ Passed Issue #71 requires a same-path guard before file operations, a concise CLI error, source preservation, retained behavior for other initialization modes, and regression tests. init_workspace() resolv…
Out of Scope Changes check ✅ Passed The reviewed changes add only the curriculum-source guard and unit and integration regression tests. The implementation and tests directly support issue #71. No unrelated production behavior or unrela…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit guards the source-tree door
No copy hops across the floor
The error speaks, the files stay still
Tests watch the path with careful skill
Safe initialization wins the thrill

Comment @coderabbitai help to get the list of available commands.

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.

cli: reject init --force at the curriculum source

1 participant