Add startup validation for required LFTP config fields (#310)

[nitrobass24]

Summary

• Add _validate_config() to Controller that checks all required config fields (Lftp, Controller, General, AutoQueue, Args) are non-None at startup, raising a single ControllerError listing all missing fields
• Add backward-compat guard in _build_pair_contexts for when no path pairs are configured and remote_path/local_path are unset, with a clear error message
• Remove type: ignore[arg-type] comments on backward-compat path now that None is explicitly checked

Closes #310

Test plan

• 8 new unit tests in test_validate_config.py covering:
  • Valid config does not raise
  • Missing single Lftp field raises with field name
  • Missing multiple fields lists all in error message
  • Missing Args.local_path_to_scanfs raises
  • Missing AutoQueue.auto_delete_remote raises
  • Missing Controller.interval_ms_local_scan raises
  • Backward-compat mode without remote_path raises
  • Backward-compat mode without local_path raises
• All existing controller unit tests pass
• Ruff format + check pass
• Pyright passes with 0 errors

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Startup now enforces required configuration fields and reports all missing items together in a single clear error.
  • Fallback path handling now validates remote/local path values and fails with an explicit error when absent.

• Tests

  • Added unit tests covering startup configuration validation and backward-compatible fallback path scenarios.

[coderabbitai]

Warning

Rate limit exceeded

@nitrobass24 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 43 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d07dd793-5516-4fc5-9efd-12288196e675

📥 Commits

Reviewing files that changed from the base of the PR and between 3c83762 and bec2274.

📒 Files selected for processing (1)

• src/python/controller/controller.py

📝 Walkthrough

Walkthrough

Added startup validation to Controller: Controller._validate_config() is invoked in Controller.__init__ to assert required config fields are present and raise a consolidated ControllerError listing missing items. Also added explicit validation for fallback remote_path/local_path in _build_pair_contexts.

Changes

Cohort / File(s)	Summary
Controller Validation src/python/controller/controller.py	Added Controller._validate_config() and call in Controller.__init__; collects and reports all missing required config fields (Lftp, Controller intervals, General.verbose, AutoQueue.auto_delete_remote, Args.local_path_to_scanfs). Validated remote_path/local_path in _build_pair_contexts before using fallbacks (removed prior ignored arg-type usage).
Validation Tests src/python/tests/unittests/test_controller/test_validate_config.py	New unit tests and helpers (_make_config, _make_args, _make_context) covering successful construction, single/multiple missing-field errors, missing Args/AutoQueue/Controller fields, and backward-compat behavior when _build_pair_contexts yields no path pairs (missing remote_path/local_path cases).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰✨ I hopped through configs, keen and spry,
Nosed out blanks that tried to hide.
I listed misses, clear and neat,
So Controller boots on certain feet.
A tiny thump — validation complete! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding startup validation for required LFTP config fields, directly aligned with the changeset.
Linked Issues check	✅ Passed	The pull request fully addresses issue #310 by implementing _validate_config() method to validate required fields, raising ControllerError with missing field names, and removing type: ignore comments.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #310: the validation implementation in controller.py and comprehensive unit tests for the validation logic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/validate-lftp-config

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[nitrobass24]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/python/controller/controller.py`:
- Around line 348-352: The error message raised now uses a generic message even
when only one of Lftp.remote_path or Lftp.local_path is missing; update the
check in the controller (where it currently tests `remote_path is None or
local_path is None`) to compute which specific field(s) are missing (e.g., build
a missing_fields list for "remote_path" and "local_path") and raise
ControllerError with a message that names the exact missing field(s) (e.g.,
"Missing configuration: remote_path" or "Missing configuration: remote_path,
local_path") so users see the precise fallback(s) that are absent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4a7667bc-17a5-4487-a2ec-5749e079b771

📥 Commits

Reviewing files that changed from the base of the PR and between 1fa5dee and 85c9963.

📒 Files selected for processing (2)

• src/python/controller/controller.py
• src/python/tests/unittests/test_controller/test_validate_config.py

[coderabbitai]

♻️ Duplicate comments (1)

src/python/controller/controller.py (1)

355-357: ⚠️ Potential issue | 🟡 Minor

Fallback error text still implies both fields are missing.

At Line 356, the message says to set remote_path and local_path even when only one field is missing (the missing list at Lines 349-353 is already precise). Consider making the guidance field-agnostic.

💡 Suggested wording adjustment

                 raise ControllerError(
                     "No path pairs configured and {} not set. "
-                    "Configure at least one path pair in Settings, or set remote_path and local_path.".format(
+                    "Configure at least one path pair in Settings, or set the missing field(s).".format(
                         ", ".join(missing)
                     )
                 )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/python/controller/controller.py` around lines 355 - 357, The error string
that uses ", ".join(missing) still instructs users to "set remote_path and
local_path" even when only one field is missing; update the message construction
around the join(missing) usage so the guidance is field-agnostic (e.g.,
reference the joined missing fields directly or say "set the missing field(s):
{missing_fields}") and ensure the code path that builds this message (the
variable/format call where missing is joined) emits the accurate, pluralized
instruction rather than always naming both remote_path and local_path.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/python/controller/controller.py`:
- Around line 355-357: The error string that uses ", ".join(missing) still
instructs users to "set remote_path and local_path" even when only one field is
missing; update the message construction around the join(missing) usage so the
guidance is field-agnostic (e.g., reference the joined missing fields directly
or say "set the missing field(s): {missing_fields}") and ensure the code path
that builds this message (the variable/format call where missing is joined)
emits the accurate, pluralized instruction rather than always naming both
remote_path and local_path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 833959b6-ccd5-4622-b12d-99159bf342bc

📥 Commits

Reviewing files that changed from the base of the PR and between 7aa9378 and 3c83762.

📒 Files selected for processing (1)

• src/python/controller/controller.py