docs(roadmap): add config model type silent ignore

This commit is contained in:
Yeachan-Heo
2026-05-20 23:01:22 +00:00
parent 07a12d4cf3
commit fbd2f01347

View File

@@ -6583,3 +6583,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
513. **`make_patch` is not a diff hunk generator; it emits the entire old file as removed lines plus the entire new file as added lines, so `structured_patch` itself can double the output size for every write/edit** — dogfooded 2026-05-20 from the `#clawcode-building-in-public` 22:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@b3b9eb2`. Code inspection: `runtime/src/file_ops.rs::make_patch(original, updated)` builds one `StructuredPatchHunk` by iterating `for line in original.lines() { lines.push(format!("-{line}")); }` and then `for line in updated.lines() { lines.push(format!("+{line}")); }`; `old_lines` and `new_lines` are the full line counts. This means the supposedly structured patch is a whole-file delete+whole-file add, not a minimal/contextual diff. Combined with #509's full `content`/`original_file` fields, editing a 1MB file can return: full original file, full new file, and a `structured_patch` containing full original+full new again. Even after removing raw content fields, `structured_patch` would still be an output-amplification bug unless it becomes a real bounded diff. **Required fix shape:** (a) replace `make_patch` with a real line diff/hunk algorithm that emits only changed hunks plus configurable context; (b) cap patch lines/bytes with `patch_truncated`, `omitted_hunks`, and full old/new byte counts; (c) for full-file rewrites, return a summary (`rewrite:true`, changed line counts, previews) rather than every line; (d) add regressions for one-line edit in a 10k-line file proving patch output is O(changed lines + context), not O(file size). **Why this matters:** structured patches are the right contract for coding tools, but a whole-file pseudo-patch creates the same context-window blowup as raw file echoes while looking machine-friendly. Review/debug loops need concise, truthful diffs. Source: gaebal-gajae dogfood response to Clawhip message `1506778574143754422` on 2026-05-20.
514. **`write_file` silently overwrites existing binary/non-UTF8 files while reporting them as creates because the previous-file read uses `fs::read_to_string(...).ok()` and drops decode/errors** — dogfooded 2026-05-20 from the `#clawcode-building-in-public` 22:30 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@ea29fbe`. Code inspection: `runtime/src/file_ops.rs::write_file` enforces only the new content byte length, resolves the destination, then does `let original_file = fs::read_to_string(&absolute_path).ok();`. Any read error — file missing, permission denied, directory race, or existing binary/non-UTF8 content — is collapsed to `None`. The function then creates parents and `fs::write(&absolute_path, content)?`. Because `kind` is computed as `if original_file.is_some() { "update" } else { "create" }`, overwriting a binary file is reported as a create with no warning and no binary guard. `read_file` has binary detection; `write_file` does not apply it before clobbering existing files. **Required fix shape:** (a) distinguish `NotFound` from other read/decode errors instead of using `.ok()`; (b) if the destination exists and is not valid UTF-8 or appears binary, require an explicit `overwrite_binary:true` / `force:true` flag or return `kind:"binary_overwrite_refused"`; (c) report `existed:true` based on metadata, not successful UTF-8 decoding; (d) preserve structured diff only for text files; (e) add regressions for overwriting a binary file and a permission-denied/unreadable file proving the tool does not silently treat them as creates. **Why this matters:** write tools are allowed to create/update source artifacts, but silently clobbering a binary asset or unreadable file is data-lossy and misleading. The model/operator needs to know whether a file existed and whether a safe text diff is possible before replacement. Source: gaebal-gajae dogfood response to Clawhip message `1506786124176293919` on 2026-05-20.
515. **Config `model` values are parsed with `JsonValue::as_str` and otherwise silently ignored, so non-string model config falls back to defaults with no error or warning** — dogfooded 2026-05-20 from the `#clawcode-building-in-public` 23:00 UTC nudge on `/home/bellman/Workspace/claw-code-pr2967` with branch/origin `docs/roadmap-workdir-provenance@07a12d4`, following Jobdori's config-loader diagnostic sweep. Code inspection: after validation/merge, `ConfigLoader::load` builds `RuntimeFeatureConfig { model: parse_optional_model(&merged_value), ... }`. `parse_optional_model` is `root.as_object().and_then(|object| object.get("model")).and_then(JsonValue::as_str).map(ToOwned::to_owned)`. If a config file contains `{"model": 123}`, `{"model": null}`, `{"model": ["opus"]}`, or an object, `parse_optional_model` returns `None` exactly as if no model key existed. Other config fields use typed helpers that return `ConfigError::Parse` on wrong types (`permissions.defaultMode`, plugin fields, hooks, etc.), so `model` is an outlier. **Required fix shape:** (a) replace `parse_optional_model -> Option<String>` with `Result<Option<String>, ConfigError>`; (b) when `model` exists but is not a non-empty string, emit `ConfigError::Parse` or a structured warning with path/key/type; (c) run the same model-syntax validator used for `--model` and env model values so config, env, and CLI flag agree; (d) expose `model_source`, `model_raw`, and validation diagnostics in status/config JSON; (e) add regressions for numeric/null/array/object model values proving they are not silently treated as missing. **Why this matters:** model selection is control-plane state. A typo like `model: ["opus"]` should not silently revert to the default model while status appears healthy; that creates prompt misdelivery and cost surprises that are hard to attribute back to config. Source: gaebal-gajae dogfood response to Clawhip message `1506793682257580144` on 2026-05-20.