feat: add typed CoreConfig dataclass with validation (CIV-009)

[F-X64]

Summary

• Added core/config.py which now contains the CoreConfig dataclass. This replaces the raw-dict approach in lib/config_lib.py (not removed yet)
• Includes functionality for converting from legacy dicts
• Validation runs automatically on construction. ValueError is raised on missing required fields, invalid environment values, or wrong types

[sourcery-ai]

Hey - I've found 4 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="core/config.py" line_range="30-50" />
<code_context>
+    test_suites: list[str] | None = None
+    include_markers: str | None = None
+
+    def __post_init__(self) -> None:
+        self.validate()
+
+    def validate(self) -> None:
+        for name in _REQUIRED_FIELDS:
+            value = getattr(self, name)
+            if value is None or (isinstance(value, str) and not value.strip()):
+                raise ValueError(
+                    f"'{name}' is required and must be a non-empty string"
+                )
+
+        if self.environment not in _VALID_ENVIRONMENTS:
</code_context>
<issue_to_address>
**issue (bug_risk):** `validate` does not enforce the declared types for most fields: non-string required paths are accepted, arbitrary values are accepted for `debug`, `parallel`, `stop_cleanup`, and path fields, and `tags` values or `test_suites` elements are not checked. Invalid configurations therefore pass construction and fail later at consumers such as file opening or `join`, instead of raising the promised `ValueError`.

**Triggers:** When a YAML or legacy dictionary contains a wrong scalar type, a non-string tag value, or a non-string test-suite entry.

**Suggested fix:** Validate every field explicitly, including string elements and tag key/value types, and reject non-boolean values for boolean fields.
</issue_to_address>

### Comment 2
<location path="core/config.py" line_range="41-45" />
<code_context>
+                    f"'{name}' is required and must be a non-empty string"
+                )
+
+        if self.environment not in _VALID_ENVIRONMENTS:
+            raise ValueError(
+                f"'environment' must be one of {sorted(_VALID_ENVIRONMENTS)}, "
+                f"got '{self.environment}'"
+            )
+
+        if self.tags is not None and not isinstance(self.tags, dict):
</code_context>
<issue_to_address>
**issue (bug_risk):** An unhashable invalid environment value, such as a YAML list, makes `self.environment not in _VALID_ENVIRONMENTS` raise `TypeError` rather than the documented `ValueError` for wrong types.

**Triggers:** When the environment field is supplied as a YAML sequence or another unhashable object.

**Suggested fix:** Check that `environment` is a string before membership testing and raise the configured `ValueError` for all invalid types.

```suggestion
        if not isinstance(self.environment, str) or self.environment not in _VALID_ENVIRONMENTS:
            raise ValueError(
                f"'environment' must be one of {sorted(_VALID_ENVIRONMENTS)}, "
                f"got '{self.environment}'"
            )
```
</issue_to_address>

### Comment 3
<location path="core/config.py" line_range="22-25" />
<code_context>
+    debug: bool = False
+    parallel: bool = False
+    stop_cleanup: bool = False
+    instances_json: str = "/tmp/civ-instances.json"
+    ssh_identity_file: str = "/tmp/civ-ssh-key"
+    ssh_pub_key_file: str = "/tmp/civ-ssh-key.pub"
+    ssh_config_file: str = "/tmp/civ-ssh-config"
+    test_filter: str | None = None
</code_context>
<issue_to_address>
**issue (broader_impact):** The new default runtime paths differ from the defaults in the existing `CIVConfig`: instances and SSH files move from `/tmp/instances.json`, `/tmp/ssh_key`, `/tmp/ssh_key.pub`, and `/tmp/ssh_config` to different names. Using `CoreConfig` as the replacement therefore makes existing consumers, attach workflows, and external tooling look at different files.

**Triggers:** When callers migrate from `CIVConfig.get_default_config()` to `CoreConfig` without explicitly setting these paths.

**Suggested fix:** Preserve the legacy default paths or provide an explicit compatibility/migration policy for the changed defaults.

```suggestion
    instances_json: str = "/tmp/instances.json"
    ssh_identity_file: str = "/tmp/ssh_key"
    ssh_pub_key_file: str = "/tmp/ssh_key.pub"
    ssh_config_file: str = "/tmp/ssh_config"
```
</issue_to_address>

### Comment 4
<location path="core/config.py" line_range="61" />
<code_context>
+        return asdict(self)
+
+    @classmethod
+    def from_dict(cls, data: dict[str, object]) -> CoreConfig:
+        for name in _REQUIRED_FIELDS:
+            if name not in data or data[name] is None:
+                raise ValueError(
+                    f"'{name}' is required and must be a non-empty string"
+                )
+
+        known = {f.name for f in fields(cls)}
+        filtered: dict[str, object] = {}
+        for key, value in data.items():
+            if key not in known:
+                continue
+            if key == "stop_cleanup" and value is None:
+                value = False
+            filtered[key] = value
+
+        return cls(**filtered)
+
+    @classmethod
</code_context>
<issue_to_address>
**issue (bug_risk):** `from_dict` assumes its argument is a mapping and calls membership and `.items()` directly, so a wrong input type raises `TypeError` or `AttributeError` instead of the promised `ValueError` for invalid configuration input.

**Triggers:** When a caller passes a list, scalar, or other non-mapping object to `CoreConfig.from_dict`.

**Suggested fix:** Check `isinstance(data, dict)` at the start of `from_dict` and raise a descriptive `ValueError` otherwise.

```suggestion
    def from_dict(cls, data: dict[str, object]) -> CoreConfig:
        if not isinstance(data, dict):
            raise ValueError(
                f"Configuration must be a dict, got {type(data).__name__}"
            )
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sshmulev]

The PR looks good, didn't find anything important to raise.
LGTM+1