Fix: file-read fail-loud for config + deps (unreadable-but-present files silently degraded) (#325)

* Fix: file-read fail-loud for config + deps — unreadable-but-present files no longer silently degrade

Audit Batch 3 (config & deps half) — three "a file that EXISTS but can't be read
must fail loud, not silently degrade" defects:

- **[medium] Unreadable config silently reverts to defaults.** load_toml_config /
  load_json_config mapped a read failure to SpecSyncConfig::default() with no
  signal, downgrading enforcement (strict→warn, exit 1→0) on a config that was
  present but not valid UTF-8. Now, since callers reach these only after an
  `.exists()` check, a None on a still-existing file warns loudly (naming the file)
  before falling back. Same for the optional local override (config.local.toml).
- **[low] deps drops an unreadable source's imports.** check_undeclared_imports did
  `Err(_) => continue`, so a non-UTF-8 declared source contributed no imports and
  `deps --strict` could pass while hiding undeclared-import violations. Now pushes a
  hard error (mirrors validator.rs's source-read policy); cmd_deps exits 1.
- **[medium] deps drops an unreadable spec from the graph.** build_dep_graph did
  `Err(_) => continue`, silently removing the node — defeating cycle and
  missing-dependency detection for that module. Refactored into an internal
  build_dep_graph_checked that also returns read-failure messages; validate_deps
  extends report.errors with them (build_dep_graph keeps its signature for the
  non-gating visualization / topological-order callers). cmd_deps exits 1.

Reproduced: non-UTF-8 config → loud "exists but could not be read" warning (was
silent); non-UTF-8 declared source → deps error + exit 1 (was exit 0); non-UTF-8
spec → deps error + exit 1 (was exit 0, node silently dropped).

Tests: 3 integration regressions (deps unreadable source, deps unreadable spec,
config unreadable warning). Documented as config spec invariant #9 and deps spec
invariant #7. 750 unit + 180 integration, fmt / clippy / self-check 100%.

(The schema-discovery fail-open — build_schema / get_schema_table_names silently
skipping unreadable migration files — is the same theme but needs signature changes
across schema.rs + validator.rs + check; it ships as its own follow-up PR.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KDJxU4R8hUEuq1Y5jzft5m

* Update: sync deps/config specs with fail-loud behavior (review changes)

Addresses Kyntrin's CHANGES_REQUESTED on #325:
- deps.spec.md: Error Cases table now reflects hard-error behavior for
  unreadable declared source AND spec files (was "Skipped during import
  extraction"); added unreadable-spec row. Version bump 2->3 + Change Log.
- config.spec.md: Error Cases table made explicit that a present-but-
  unreadable config warns loudly before falling back. Version bump 1->2 +
  Change Log.

specsync check --strict passes (60/60).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: 0xLeif

specs/config/config.spec.md

@@ -1,6 +1,6 @@
 ---
 module: config
-version: 1
+version: 2
 status: stable
 files:
   - src/config.rs
@@ -60,6 +60,7 @@ The configuration file supports the following top-level sections:
 6. Root-level source files (no subdirectories) produce `["."]` as source dirs
 7. TOML parsing is zero-dependency — uses line-by-line string parsing, not a TOML library
 8. The reader accepts both TOML string kinds for scalar and array values: basic `"..."` strings (backslash escapes decoded) and literal `'...'` strings (taken verbatim, no escape processing); a `#`, `,`, `[`, or `]` appearing inside either kind is treated as content, not as a comment or array structure
+9. A config file that is absent is expected — defaults apply silently. But a config file that **exists yet cannot be read** (e.g. not valid UTF-8) fails loud: a warning naming the file is printed and built-in defaults are used, rather than silently reverting to defaults (which would downgrade enforcement — strict→warn, exit 1→0 — with no signal). The same applies to the optional local override file (`config.local.toml`)
 
 ## Behavioral Examples
 
@@ -85,7 +86,8 @@ The configuration file supports the following top-level sections:
 
 | Condition | Behavior |
 |-----------|----------|
-| Config file unreadable | Falls back to `SpecSyncConfig::default()` |
+| Config file exists but unreadable (e.g. not valid UTF-8) | Prints a warning naming the file to stderr, then falls back to `SpecSyncConfig::default()` (fail-loud, not silent) |
+| Config file absent | Silently uses `SpecSyncConfig::default()` with auto-detected source dirs (expected) |
 | Malformed JSON config | Prints warning to stderr, falls back to defaults |
 | Empty project root | Returns `["src"]` as source dirs |
 
@@ -116,3 +118,4 @@ The configuration file supports the following top-level sections:
 | 2026-03-28 | Document discover_manifest_modules |
 | 2026-04-06 | Document github config section, rules section, and full config file structure |
 | 2026-07-03 | Document `read_config_file` (leading-BOM-tolerant config read, shared with `migrate`) |
+| 2026-07-06 | Document fail-loud behavior for present-but-unreadable config (invariant 9): unreadable config/local-override files now warn loudly before falling back to defaults; clarified Error Cases table |

specs/deps/deps.spec.md

@@ -1,6 +1,6 @@
 ---
 module: deps
-version: 2
+version: 3
 status: stable
 files:
   - src/deps.rs
@@ -45,6 +45,7 @@ Cross-module dependency validation. Parses `depends_on` declarations from spec f
 4. Cross-project refs (containing `/`) in `depends_on` are skipped — only local deps are validated
 5. Undeclared imports (found in source but not in `depends_on`) are reported as warnings, not errors
 6. Module names in `depends_on` paths are extracted from the path's directory component
+7. A spec or declared source file that exists but cannot be read as UTF-8 is a hard error (not a silent skip): an unreadable spec would otherwise be dropped from the graph — defeating cycle and missing-dependency detection for that module — and an unreadable source would contribute no imports, hiding undeclared-import violations. Both are recorded in `report.errors`, so `cmd_deps` exits 1 rather than under-validating silently
 
 ## Behavioral Examples
 
@@ -70,7 +71,8 @@ Cross-module dependency validation. Parses `depends_on` declarations from spec f
 
 | Condition | Behavior |
 |-----------|----------|
-| Source file unreadable | Skipped during import extraction |
+| Declared source file exists but unreadable as UTF-8 | Hard error recorded in `report.errors`; `cmd_deps` exits 1 (not skipped during import extraction) |
+| Spec file exists but unreadable as UTF-8 | Hard error recorded in `report.errors`; node is not silently dropped from the graph; `cmd_deps` exits 1 |
 | Spec frontmatter unparseable | Module excluded from dependency graph |
 | No specs found in specs_dir | Returns empty graph and clean DepsReport |
 
@@ -94,5 +96,6 @@ Cross-module dependency validation. Parses `depends_on` declarations from spec f
 
 | Date | Change |
 |------|--------|
+| 2026-07-06 | Documented fail-loud behavior for unreadable declared source and spec files: added invariant 7 and updated Error Cases table so an existing-but-non-UTF-8 file is a hard error gating `cmd_deps` rather than a silent skip |
 | 2026-04-10 | Populated requirements.md with user stories, acceptance criteria, constraints, and out-of-scope items |
 | 2026-04-07 | Initial spec |

src/config.rs

@@ -249,7 +249,19 @@ pub fn load_config(root: &Path) -> SpecSyncConfig {
 fn merge_local_config(local_path: &Path, config: &mut SpecSyncConfig) {
     let content = match read_config_file(local_path) {
         Some(c) => c,
-        None => return,
+        None => {
+            // Local config is optional, but a file that exists yet cannot be read
+            // (not valid UTF-8) must not be silently ignored — its per-developer
+            // overrides (ai_provider/ai_model/etc.) would be dropped with no signal.
+            if local_path.exists() {
+                eprintln!(
+                    "Warning: local config {} exists but could not be read (not valid UTF-8 or unreadable); \
+                     per-developer overrides are NOT applied",
+                    local_path.display()
+                );
+            }
+            return;
+        }
     };
 
     let mut current_section: Option<String> = None;
@@ -695,7 +707,18 @@ const KNOWN_JSON_KEYS: &[&str] = &[
 fn load_json_config(config_path: &Path, root: &Path) -> SpecSyncConfig {
     let content = match read_config_file(config_path) {
         Some(c) => c,
-        None => return SpecSyncConfig::default(),
+        None => {
+            // See load_toml_config: a present-but-unreadable config must fail loud
+            // rather than silently downgrade enforcement to defaults.
+            if config_path.exists() {
+                eprintln!(
+                    "Warning: config file {} exists but could not be read (not valid UTF-8 or unreadable); \
+                     using built-in defaults — its settings (enforcement, required sections, etc.) are NOT applied",
+                    config_path.display()
+                );
+            }
+            return SpecSyncConfig::default();
+        }
     };
 
     // Warn about unknown keys
@@ -742,7 +765,22 @@ fn load_json_config(config_path: &Path, root: &Path) -> SpecSyncConfig {
 fn load_toml_config(config_path: &Path, root: &Path) -> SpecSyncConfig {
     let content = match read_config_file(config_path) {
         Some(c) => c,
-        None => return SpecSyncConfig::default(),
+        None => {
+            // read_config_file returns None for both "absent" and "present but
+            // unreadable". This function is reached only after an `.exists()` check
+            // (load_config) or with a known migration source path, so a None on a
+            // file that still exists means it could not be read (e.g. not valid
+            // UTF-8) — fail loud instead of silently reverting to defaults, which
+            // would downgrade enforcement (strict→warn, exit 1→0) with no signal.
+            if config_path.exists() {
+                eprintln!(
+                    "Warning: config file {} exists but could not be read (not valid UTF-8 or unreadable); \
+                     using built-in defaults — its settings (enforcement, required sections, etc.) are NOT applied",
+                    config_path.display()
+                );
+            }
+            return SpecSyncConfig::default();
+        }
     };
 
     let mut config = SpecSyncConfig::default();

src/deps.rs

@@ -69,14 +69,39 @@ pub struct DepsReport {
 
 /// Build the dependency graph from all spec files in the project.
 pub fn build_dep_graph(root: &Path, specs_dir: &str) -> HashMap<String, DepNode> {
+    build_dep_graph_checked(root, specs_dir).0
+}
+
+/// Like `build_dep_graph`, but also returns error messages for spec files that
+/// EXIST yet could not be read as UTF-8. Silently dropping such a spec removes its
+/// node from the graph, which defeats cycle detection and missing-dependency
+/// checks for that module — so the validation path (`validate_deps`) surfaces
+/// these as hard errors rather than losing them. Kept internal; `build_dep_graph`
+/// preserves the original signature for the non-gating callers (visualization,
+/// topological order).
+fn build_dep_graph_checked(
+    root: &Path,
+    specs_dir: &str,
+) -> (HashMap<String, DepNode>, Vec<String>) {
     let specs_path = root.join(specs_dir);
     let spec_files = find_spec_files(&specs_path);
     let mut graph: HashMap<String, DepNode> = HashMap::new();
+    let mut unreadable: Vec<String> = Vec::new();
 
     for spec_file in &spec_files {
         let content = match fs::read_to_string(spec_file) {
             Ok(c) => c.replace("\r\n", "\n"),
-            Err(_) => continue,
+            Err(err) => {
+                let rel = spec_file
+                    .strip_prefix(root)
+                    .unwrap_or(spec_file)
+                    .to_string_lossy()
+                    .to_string();
+                unreadable.push(format!(
+                    "{rel}: spec file could not be read as UTF-8; dependency analysis skipped this spec: {err}"
+                ));
+                continue;
+            }
         };
 
         let parsed = match parse_frontmatter(&content) {
@@ -117,7 +142,7 @@ pub fn build_dep_graph(root: &Path, specs_dir: &str) -> HashMap<String, DepNode>
         );
     }
 
-    graph
+    (graph, unreadable)
 }
 
 /// Extract a module name from a dependency path.
@@ -146,8 +171,11 @@ fn extract_module_from_dep_path(dep: &str) -> Option<String> {
 
 /// Validate the entire dependency graph.
 pub fn validate_deps(root: &Path, specs_dir: &str) -> DepsReport {
-    let graph = build_dep_graph(root, specs_dir);
+    let (graph, unreadable_specs) = build_dep_graph_checked(root, specs_dir);
     let mut report = DepsReport::default();
+    // A spec that exists but couldn't be read was dropped from the graph; record it
+    // as a hard error so cmd_deps exits 1 instead of silently under-validating.
+    report.errors.extend(unreadable_specs);
 
     let known_modules: HashSet<&str> = graph.keys().map(|k| k.as_str()).collect();
     report.module_count = graph.len();
@@ -326,7 +354,18 @@ fn check_undeclared_imports(
             let full_path = root.join(file);
             let content = match fs::read_to_string(&full_path) {
                 Ok(c) => c,
-                Err(_) => continue,
+                // A declared source file that can't be read as UTF-8 silently
+                // contributed no imports, so `deps --strict` could pass while
+                // hiding real undeclared-import violations. Fail loud instead —
+                // mirrors the validator's source-read policy (validator.rs) and,
+                // because cmd_deps exits 1 on any error, gates CI consistently.
+                Err(err) => {
+                    report.errors.push(format!(
+                        "{}: source file `{}` could not be read as UTF-8 for dependency analysis: {}",
+                        node.spec_path, file, err
+                    ));
+                    continue;
+                }
             };
             let file_imports = extract_imports(&full_path, &content);
             actual_imports.extend(file_imports);

tests/integration.rs

@@ -6455,6 +6455,78 @@ fn deps_strict_passes_when_dependency_is_declared() {
         .success();
 }
 
+#[test]
+fn deps_fails_loud_on_unreadable_source_file() {
+    // Regression: a declared source file that can't be read as UTF-8 silently
+    // contributed no imports, so `deps` could pass while hiding undeclared imports.
+    let tmp = TempDir::new().unwrap();
+    let root = tmp.path();
+    write_config(root, "specs", &["src"]);
+    fs::create_dir_all(root.join("src")).unwrap();
+    let mut bad = b"export function apiFn() {}\n".to_vec();
+    bad.push(0xFF);
+    fs::write(root.join("src/a.ts"), bad).unwrap();
+    fs::create_dir_all(root.join("specs/m")).unwrap();
+    fs::write(
+        root.join("specs/m/m.spec.md"),
+        valid_spec("m", &["src/a.ts"]),
+    )
+    .unwrap();
+
+    specsync()
+        .current_dir(root)
+        .arg("deps")
+        .assert()
+        .failure()
+        .stdout(predicate::str::contains(
+            "could not be read as UTF-8 for dependency analysis",
+        ));
+}
+
+#[test]
+fn deps_fails_loud_on_unreadable_spec_file() {
+    // Regression: a spec file that can't be read as UTF-8 was silently dropped from
+    // the dependency graph, defeating cycle / missing-dep detection for that module.
+    let tmp = TempDir::new().unwrap();
+    let root = tmp.path();
+    write_config(root, "specs", &["src"]);
+    fs::create_dir_all(root.join("src")).unwrap();
+    fs::write(root.join("src/a.rs"), "pub fn f() {}\n").unwrap();
+    fs::create_dir_all(root.join("specs/m")).unwrap();
+    let mut bad = b"---\nmodule: m\nfiles:\n  - src/a.rs\n---\n# m\n".to_vec();
+    bad.push(0xFF);
+    fs::write(root.join("specs/m/m.spec.md"), bad).unwrap();
+
+    specsync()
+        .current_dir(root)
+        .arg("deps")
+        .assert()
+        .failure()
+        .stdout(predicate::str::contains(
+            "spec file could not be read as UTF-8",
+        ));
+}
+
+#[test]
+fn config_warns_on_unreadable_config_file() {
+    // Regression: a config file that exists but can't be read as UTF-8 silently
+    // reverted to built-in defaults, downgrading enforcement with no signal.
+    let tmp = TempDir::new().unwrap();
+    let root = tmp.path();
+    fs::create_dir_all(root.join("src")).unwrap();
+    fs::write(root.join("src/a.rs"), "pub fn f() {}\n").unwrap();
+    // A config whose keys are valid ASCII but whose tail is invalid UTF-8.
+    let mut bad = b"specs_dir = \"specs\"\nsource_dirs = [\"src\"]\n".to_vec();
+    bad.extend_from_slice(&[0xFF, 0xFE]);
+    fs::write(root.join(".specsync.toml"), bad).unwrap();
+
+    specsync()
+        .current_dir(root)
+        .arg("check")
+        .assert()
+        .stderr(predicate::str::contains("exists but could not be read"));
+}
+
 #[test]
 fn deps_strict_mermaid_still_gates() {
     // Regression: `deps --mermaid`/`--dot` early-returned before the strict gate, so