fix(spec): make the config block survive being written out

[jdx]

First of the config v2 stack. Four defects in config { prop … }, all in the ground v2 builds on.

Nothing renders the block into docs yet (lib/src/docs/models.rs:12 has // pub config: SpecConfig, commented out), but the SDK generators do consume it — and their workarounds are the evidence for two of these.

data_type was parsed and never written

It survived exactly one hop:

prop "jobs" data_type="integer"   →  parse → Integer → write → (nothing) → parse → Null

SpecDataTypes gains a Display (the KDL spelling) and the writer emits it.

default stored KDL source text, not a value

It kept KdlValue::to_string(), so default=#true was read as the five characters #true and default="4" as "4" with its quotes — and writing it out added another pair, one layer per round trip. The old snapshot in this file recorded exactly that (default="#true", default="4"), which is how the bug stayed.

It's a typed SpecConfigValue now (bool/int/float/string), which is also what the v2 vocabulary needs for typed defaults. The Python SDK generator had been undoing the damage by hand:

SpecDataTypes::Boolean => match d.as_str() {
    "#true" | "true" => " = True".to_string(),      // the leaked KDL spelling
    …
SpecDataTypes::Integer | SpecDataTypes::Float => {
    let numeric = d.trim_matches('"');              // the added quotes

Both gone; it reads the value and renders by the declared data_type where there is one — which keeps the data_type=float default="1.5" case rendering as 1.5 rather than "1.5" (my first attempt at this regressed that, caught by its snapshot).

A nested prop block parsed and lost its children

prop "status" { prop "missing_tools" } kept status and silently dropped the rest — the property loop reads properties only. Refused now, naming the dotted spelling to use instead.

merge was first-wins per prop

Everything else in Spec::merge is other-wins (merge_opt!); config was the one place an include could add a prop but never correct one. Last-wins now, whole prop at a time.

Verification

A test per defect, and I checked each one fails when its fix is reverted (four mutations, four failures). Full workspace green, clippy clean, mise run render produces no diff.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.

---

Note

Medium Risk
Changes spec parsing, serialization, and generated SDK code paths where bad defaults could previously break imports or silently corrupt values; new validation may reject previously tolerated specs.

Overview
Config prop defaults are typed values now, not KDL source strings, so parse → write → parse keeps booleans, numbers, and strings without extra quoting layers. The writer also emits data_type again (via Display on SpecDataTypes), and defaults go out through typed KDL entries with string_entry for strings so control characters still reparse.

Parsing is stricter: nested prop { … } is rejected, defaults must fit the declared type (and finite floats / i64 range), with coercion after all props on the node are read. SpecConfig::merge is last-wins per prop, matching other Spec::merge behavior.

SDK output: shared escape_string_literal escapes control characters for Python/TS; the Python config dataclass renders defaults from SpecConfigValue (quoted strings only) instead of #true/quote-stripping hacks, with a test that malicious string defaults cannot become import-time code.

^{Reviewed by Cursor Bugbot for commit 706e618. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Improvements
  • Configuration defaults now preserve native types, including booleans, integers, decimals, and strings.
  • Generated Python configuration values use correctly typed, safely formatted defaults.
  • String values are safely escaped across generated Python and TypeScript output.
  • Configuration serialization and round trips are more consistent and reliable.
  • Invalid nested configuration properties are rejected with clearer parse errors.
  • Repeated configuration properties now use the latest declaration.
  • Integer ranges and declared configuration types are validated more reliably.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: ada6c649-2d61-46d8-b207-e3c38687a3ee

📥 Commits

Reviewing files that changed from the base of the PR and between ee5da6f and 706e618.

📒 Files selected for processing (3)

• lib/src/sdk/mod.rs
• lib/src/sdk/python/mod.rs
• lib/src/spec/config.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• lib/src/sdk/python/mod.rs
• lib/src/spec/config.rs

---

📝 Walkthrough

Walkthrough

Configuration defaults now use typed values instead of source strings. KDL parsing validates and preserves typed defaults, serialization emits native values, and merging applies later declarations. Python and TypeScript string literals use shared escaping.

Changes

Typed configuration defaults

Layer / File(s)	Summary
Typed value contract lib/src/spec/config.rs, lib/src/spec/data_types.rs	Adds SpecConfigValue, updates configuration default APIs, and enables display and snake-case serialization for SpecDataTypes.
Config parsing, serialization, and merging lib/src/spec/config.rs	Parses native KDL values, validates coercions and numeric ranges, rejects nested properties, preserves typed defaults and data types during serialization, and lets later declarations replace earlier properties.
Python and TypeScript literal escaping lib/src/sdk/mod.rs, lib/src/sdk/python/mod.rs	Uses shared escaping for string literals and renders typed Python defaults as safe boolean, numeric, and string literals. Regression tests cover expression-like, quoted, and multiline values.

Estimated code review effort: 4 (Complex) | ~45 minutes

Mergeability Score: ⚪ Minimal · up to 706e6

This PR makes config defaults typed, preserves data types on write, rejects invalid nested properties and non-finite values, and changes config merges to last-wins; the stated defect-specific tests and full workspace checks pass, so no actionable merge-blocking risk remains beyond normal checks.

Sequence Diagram(s)

sequenceDiagram
  participant KDL
  participant SpecConfig
  participant PythonSDK
  KDL->>SpecConfig: Parse and validate typed default
  SpecConfig->>SpecConfig: Coerce default to declared type
  SpecConfig->>PythonSDK: Provide typed configuration property
  PythonSDK-->>KDL: Render escaped Python literal

Loading

Poem

A rabbit sorts each typed leaf,
Safe strings bring quick relief.
KDL keeps values clear,
Python prints the right form here.
Later hops replace the old!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: fixing config block serialization and persistence across write and parse operations.

---

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.}

[greptile-apps]

Greptile Summary

The PR makes config properties round-trip safely through KDL and preserves typed defaults for SDK generation.

• Adds typed boolean, integer, float, and string config defaults with explicit validation.
• Serializes data_type and config defaults without losing or progressively quoting values.
• Rejects nested config properties and applies last-declaration-wins merge behavior.
• Generates safely escaped, typed Python defaults.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; unquoted integers outside the supported range now fail parsing explicitly, and quoted out-of-range integer defaults declared as integers are also rejected after all properties have been read.

Important Files Changed

Filename	Overview
lib/src/spec/config.rs	Adds typed config values, range and type validation, lossless serialization, nested-property rejection, and replacement merge semantics; both previously reported integer-default paths are fixed.
lib/src/spec/data_types.rs	Adds stable display and serialization spellings so declared config types survive KDL round trips.
lib/src/sdk/python/mod.rs	Renders defaults from typed variants and quotes and escapes arbitrary string defaults before embedding them in generated Python.
lib/src/sdk/mod.rs	Extends shared Python and TypeScript string escaping to cover control characters in generated source literals.

_{Reviews (5): Last reviewed commit: "fix(spec): make the config block survive..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/src/sdk/python/mod.rs`:
- Around line 137-156: Update the numeric branches in the default-rendering
match within the Python field generation function to emit only validated integer
or float literals. Parse string defaults into numeric values or reject invalid
string/type combinations during spec parsing, and ensure malformed values such
as executable expressions never reach v.display() or generated Python source.

In `@lib/src/spec/config.rs`:
- Around line 27-34: Update ConfigValue::from_kdl to propagate a parse error
when converting KdlValue::Integer fails i64::try_from, instead of returning
None; reserve None exclusively for KdlValue::Null. Update the callers around the
default handling at lines 124-126 to propagate this error so out-of-range
declared defaults are not treated as absent.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: d5722d81-cf99-4a0e-aaad-a9d416ff3532

📥 Commits

Reviewing files that changed from the base of the PR and between 128d19f and f592b9c.

📒 Files selected for processing (3)

• lib/src/sdk/python/mod.rs
• lib/src/spec/config.rs
• lib/src/spec/data_types.rs

[github-actions]

Instruction counts

benchmark	trend	instructions	Δ	wall (min)	Δ
markdown	▁▁▁▁▁▁▁▁▁▂██	152,857,235 → 152,937,712	+0.05%	14.64 → 14.97ms	+2.29%
startup	▁███████████	1,201,903 → 1,202,071	+0.01%	1.04 → 0.97ms	-6.10%

No instruction-count regression above 1%.

_{Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.}

_{Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.}

Shadow comparison

Parsing mise use -g node@20 against a shadow of mise's committed spec.
Reported, not gated: the shadow grows as the derive learns to express more, so
what to watch is the ratio rather than either column.

	usage	clap	ratio
instructions, cold parse	40499	5960254	147x

usage: argv -> struct                            1803 ns      1.80 µs
clap: build tree + parse -> struct             510614 ns    510.61 µs
clap: parse -> struct, tree reused              23832 ns     23.83 µs
clap: build tree only                          312363 ns    312.36 µs

_{706e618de5df vs cca57930b536 · measured on the runner, not pushed to the history.}

[jdx]

Three real ones and one that doesn't reproduce. The Critical was the important one, and it was a consequence of my own fix in this PR.

String default emitted as a Python expression (CodeRabbit, Critical). Rendering by the declared data_type meant a data_type=integer whose default was arbitrary text wrote that text into types.py unquoted — and types.py gets imported, so default="__import__('os').system('touch /tmp/pwned')" would have run on import. Fixed at both ends:

• the spec reads a default as the declared type only where the text is that type, so data_type=float default="1.5" becomes Float(1.5) and data_type=integer default="__import__('os')" stays a String;
• the generator emits a bare literal only for a value that is one, quoting and escaping anything else.

There's an end-to-end test with that exact payload asserting the output contains no = __import__ and does contain the quoted form, plus a default containing " staying escaped.

Out-of-range integer silently dropped (greptile + CodeRabbit, same bug). KDL parses i128; try_from(…).ok() turned an out-of-range integer into "no default", losing a number somebody wrote — and then the writer and the SDKs both reported the property as having none. It's a parse error now (config default does not fit in a 64-bit integer), and None means an explicit #null.

Whole floats becoming ints (Bugbot, Medium) — does not reproduce. I measured it before and after:

default=1.0   → written: default=1.0     → back: Float(1.0)
default=2.50  → written: default=2.5     → back: Float(2.5)
default=1e3   → written: default=1000.0  → back: Float(1000.0)

kdl 6.5's float rendering already keeps the decimal point. I had written a value_repr fix for this and then found the mutation test passed without it — which is what told me the fix was unnecessary rather than untested. It's removed; the test stays as a pin, labelled as one, with the reasoning in a comment so nobody re-adds it.

All four fixes mutation-checked: revert the fix, the test fails (except the float one, which is how I caught it).

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/src/spec/config.rs`:
- Around line 27-30: Reject non-finite values at every float-construction path,
including KDL float parsing, string coercion, and
SpecConfigProp::default_value(f64), while preserving non-finite string defaults
as SpecConfigValue::String. Add regressions covering `#nan/`#inf/#-inf,
default="NaN", and successful Python module execution without emitted NaN or
inf.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ad84e22-a147-45d3-a9c7-401adaba1970

📥 Commits

Reviewing files that changed from the base of the PR and between f592b9c and ee5da6f.

📒 Files selected for processing (2)

• lib/src/sdk/python/mod.rs
• lib/src/spec/config.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• lib/src/sdk/python/mod.rs

[jdx]

No, I had not seen that one — it landed on the post-rebase commit, after my last sweep. It was right, and so were the two other findings I had missed on this PR. All three are fixed in the amended head.

String defaults skipped the round-trip helper. Exactly as described, and I reproduced it before fixing: a default containing a colour escape wrote a spec that no longer parses.

config {
    prop prompt default="a[0mb"   ← the escape went out literally
}

string_entry exists for this and the typed-default writer built its entry by hand. It goes through the helper now, as does the prop's own key.

Non-finite float defaults. #inf and #nan parse in KDL and nothing downstream can carry one. Measured, not assumed: usage g json reported "default": null — the default silently gone — because serde_json writes a non-finite float as null, and the Python generator emitted a bare inf, which is a NameError. Refused at parse now, by both roads (default=#inf and default="inf" on a float-typed prop).

Out-of-range string defaults, and the wider case behind it. Greptile's example was a 20-digit number in quotes on an integer-typed prop, which slipped past the range check the same number unquoted would have hit. The narrow fix would have been a range check on the string; instead a string the declared type cannot read is now refused outright.

That reverses a decision I made deliberately last time — keeping it as a string, so nothing would treat its text as a number — so it is worth saying why. Keeping it left the spec asserting two contradictory things, and the generator then wrote an int field defaulting to a quoted sentence. Refusing is safe and honest. I checked it refuses nothing anybody has written: across mise's 280 settings, the largest registry in the fleet, every string default reads as its declared type.

The security test moves rather than disappears. Its original case no longer parses, but a string-typed default legitimately holds arbitrary text, so the generator must still quote — two defences, and that is what it now tests.

One note on method: UsageErr::InvalidInput renders as "Invalid usage config" whatever went wrong, so the new tests assert on the diagnostic's label. A test that only checks is_err() passes just as happily when the spec was refused for an unrelated reason.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit c712e17. Configure here.}

[jdx]

Two more, both real.

Control characters broke the generated Python. escape_py_string handled backslashes and quotes only, so a default containing a newline was written into types.py unescaped and the module fails to import — a worse failure than one that merely says something wrong. Fixed in the shared escaper, which escape_ts_string was also using, so the TypeScript side and every other string these generators emit (help text included) are covered by the same change.

A native default under a declared string. type="string" default=4 left the value a number, so the field was typed str and defaulted to 4. Coercion now runs in that direction too. Every other declared type is a number or a boolean, where a bare value is already the right shape.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.