fix(toolset): round-trip serialized tool options

[atharvasingh7007]

Summary

• add a shared serialize_tool_options() helper for backend and task tool specs
• quote comma-containing string values so bracketed opts can round-trip safely
• omit empty brackets when all remaining opts are filtered out
• teach the legacy manual parser to respect quoted commas so mixed quoted/unquoted payloads still parse correctly

Root cause

BackendArg::full_with_opts() and TaskToolValue::to_tool_spec() both hand-rolled the same key=value formatting.

That formatting had two round-trip problems:

1. comma-containing string values could be split into fake extra keys on the way back in
2. when every remaining opt was an Array/Table, the task path could emit malformed empty brackets

While validating the serializer fix locally, there was one more shared-boundary issue: selectively quoting only the comma-containing value still hit the legacy manual parser whenever sibling string values stayed unquoted, so the manual parser also needed to stop splitting on commas inside quotes.

Testing

• cargo test serialize_tool_options
• cargo test full_with_opts
• cargo test to_tool_spec
• cargo test parse_tool_options

All runs were executed locally with RUSTFLAGS=-C debuginfo=0 and CARGO_INCREMENTAL=0 because this Windows machine was short on disk space during the initial build.

[gemini-code-assist]

Code Review

This pull request introduces a centralized serialize_tool_options function to handle the serialization of tool options, specifically addressing issues with comma-containing strings by applying TOML-quoting. It also updates the manual parser to correctly handle these quoted values and escaped characters. The review feedback suggests simplifying the quoting logic by leveraging the Display implementation of toml::Value and extending the set of characters that trigger quoting to include brackets, which prevents potential issues with regex-based parsers. Additionally, a redundant logic block in the escaping mechanism was identified for removal.

[greptile-apps]

Greptile Summary

This PR introduces a shared serialize_tool_options() helper that correctly quotes comma-, quote-, and bracket-containing string values for safe round-tripping, refactors both BackendArg::full_with_opts() and TaskToolValue::to_tool_spec() to use it, replaces the split_bracketed_opts regex with a quote-aware char parser so brackets inside quoted values are no longer misidentified as the opts boundary, and teaches the manual parser to respect quoted commas and TOML-decode single- and double-quoted value segments.

All four concerns raised in prior review threads (missing " quoting, single-quoted value corruption, manual parser dropping keys after single-quoted segments, and empty-bracket emission for complex-only opts) are resolved in the current commit.

Confidence Score: 5/5

Safe to merge — all previously flagged round-trip bugs are resolved and the new shared serializer is well-tested.

All four concerns from prior review threads (missing quote handling, single-quote value corruption, manual parser dropping keys after single-quoted segments, and empty-bracket emission) are addressed in the current HEAD. No new P0/P1 issues were found. The quote-aware char parsers in split_bracketed_opts and split_tool_option_segments are correct, the parse_tool_option_value TOML-decode fallback is safe, and the test suite covers key edge cases including comma-in-value, bracket-in-value, and all-complex-opts scenarios.

No files require special attention.

Important Files Changed

Filename	Overview
src/toolset/tool_version_options.rs	Core of the fix: adds serialize_tool_options/serialize_tool_option/string_requires_tool_option_quotes, rewrites manual parser to use quote-aware split_tool_option_segments, and adds parse_tool_option_value for TOML-decoding individual value segments. Well-tested with 6 new targeted unit tests.
src/cli/args/backend_arg.rs	Replaces the opts-serialization inline loop in full_with_opts() with serialize_tool_options(), and replaces the split_bracketed_opts regex with a quote-aware char parser that correctly handles brackets inside quoted option values.
src/task/mod.rs	TaskToolValue::to_tool_spec() now delegates to serialize_tool_options(), fixing silent emission of malformed brackets for Array/Table-only opts and the same comma/quote round-trip bug.
src/toolset/mod.rs	Re-exports serialize_tool_options from the public toolset API; one-line change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Tool option value] --> B{Array or Table type?}
    B -- Yes --> C[Omit - cannot round-trip]
    B -- No --> D{Needs quoting?}
    D -- has comma, bracket, or quote char --> E[TOML-format the value]
    D -- plain string --> F[Emit as-is]
    E --> G[Quoted segment]
    F --> H[Serialized opts string joined by comma]
    G --> H

    H --> I{try_parse_as_toml}
    I -- success --> J[ToolVersionOptions result]
    I -- fail --> K[parse_tool_options_manual]
    K --> L[split_tool_option_segments]
    L --> M[parse_tool_option_value]
    M --> J

    H --> N[split_bracketed_opts]
    N --> O[tool name plus opts_str]
    O --> I

Loading

_{Reviews (4): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile}

[atharvasingh7007]

Addressed the single-quote round-trip gap in 899d07eed.

What changed:

• quote string option values when they contain ' as well as ,, ", [ or ]
• added a regression test for a value like 'hi' to ensure serialize to parse preserves the surrounding single quotes

Local validation passed:

• cargo test test_serialize_tool_options_preserves_single_quote_wrapped_strings
• cargo test serialize_tool_options
• cargo test parse_tool_options