fix: strictly parse inline settings JSON instead of silently dropping it

[tanveer-arch]

Fixes #1335.

Tested against commit 755bd5a.

When sandbox is set, a malformed inline settings string (e.g. one trailing comma) was silently discarded — the run proceeded with only the sandbox settings while looking healthy — accompanied by a misleading "treating as file path" warning naming the JSON text itself. The file form raised a bare JSONDecodeError that escaped the ClaudeSDKError hierarchy (_build_command() was called one line above the try in connect(), so _exit_error was never set).

This PR, following the direction blessed in the issue:

• parses the inline settings string strictly: malformed JSON raises ValueError identifying it as the inline settings string;
• makes the file-form error name the file;
• keeps the documented missing-file degradation as-is (warning + sandbox-only settings);
• moves _build_command() inside the try in connect() so both error shapes stay inside the ClaudeSDKError hierarchy with _exit_error set.

Adds 5 regression tests covering the full behavior matrix, asserting on both the raised errors and the warning contents. Full suite: 1489 passed.

Credit to @feiiiiii5 for the detailed analysis and behavior matrix in the issue.

[feiiiiii5]

Commenting rather than submitting a formal review: I am an outside contributor and not a triager for this area, so I have no review action to submit. But since you asked, here is what I verified rather than skimmed.

At 755bd5a3 the change holds up.

What I ran

tests/test_transport.py in full: 209 passed. The wider suite is 1401 passed, 5 skipped (test_build_wheel.py and test_download_cli.py error on collection with FileNotFoundError in my environment — both shell out to packaging scripts I do not have; unrelated to your change).

ruff check and ruff format --check clean on both changed files; mypy clean on subprocess_cli.py.

I also did the A/B that matters for this issue: reverting only subprocess_cli.py to origin/main while keeping your tests gives 3 failed, 2 passed — test_malformed_inline_settings_with_sandbox_raises, test_malformed_settings_file_with_sandbox_raises_naming_file, and test_malformed_inline_settings_connect_wraps_error. So the tests genuinely pin the fix rather than passing incidentally.

Two notes, neither blocking

On ValueError vs the SDK hierarchy. Your inline and file-form errors both raise ValueError, and the docstring now documents Raises: ValueError. That reads fine to me because connect() wraps it into CLIConnectionError, and your fifth test proves the wrap happens with _exit_error set. Worth being deliberate about though: the win in the original report was that the file form escaped the ClaudeSDKError hierarchy — and for anyone calling _build_command() directly rather than going through connect(), ValueError is still what they get. That is not a regression (it was a bare JSONDecodeError, also outside the hierarchy), and arguably ValueError is the more accurate type for malformed input. I mention it only so the tradeoff is written down rather than implied.

On the missing-file degradation. test_missing_settings_file_with_sandbox_degrades passes both before and after, which is what you want — it guards against someone later "fixing" the documented degradation along with the strict parse.

One thing I would raise with the maintainers, not with you

The strict inline parse is a behaviour change: anyone whose settings string happens to contain a trailing comma today is currently running with no settings at all and will now get a hard failure. That is the right trade — silent loss is worse than a loud error — but it is the kind of change that sometimes wants a changelog entry rather than only a docstring update. Whether that is warranted is a maintainer call, not mine, so I have not commented on the PR.

Thanks for picking this up and for stating the tested commit in the description. That made the verification straightforward.

[tanveer-arch]

Thanks for the thorough verification the A/B check is a great sanity check, glad the tests pin the fix properly.

On the ValueError vs hierarchy point: agreed, it's a deliberate tradeoff. The connect() path wraps it into CLIConnectionError with _exit_error set, and for direct _build_command() callers it's no worse than the bare JSONDecodeError it replaces — arguably more accurate for malformed input.

On the changelog: happy to add an entry if the maintainers want one just let me know the preferred format/section.

[sigley]

On exact head 755bd5a, the trailing-comma case is fixed, but the same silent-drop class remains for malformed inline JSON that does not end with a closing brace.

For example, with sandbox enabled:

settings = '{model:sonnet'

_build_settings_value() does not try JSON parsing because the discriminator requires both startswith({) and endswith(}). It falls into the file-path branch, logs:

Settings file not found: {model:sonnet

and returns only the sandbox settings. I reproduced that directly on this head; the five added focused tests also pass, so this boundary is currently unpinned.

That still conflicts with the new docstring/PR claim that an inline JSON string which fails to parse raises. Could the JSON-like discriminator avoid requiring the closing brace (for example, treat a stripped value starting with { as inline and parse it strictly), with a regression for a missing closing brace? Then malformed inline settings cannot be silently reinterpreted as a missing path.

[tanveer-arch]

Good catch fixed. The discriminator now treats any stripped value starting with { as inline JSON and parses it strictly, so '{model:sonnet' raises ValueError: Invalid JSON in inline settings string instead of falling through to the file-path branch. Added test_malformed_inline_settings_missing_brace_raises pinning that boundary (asserts the "Settings file not found" warning is gone too). All settings tests pass, ruff clean.

[sigley]

Re-checked exact head 0d783b2 after the missing-brace fix. The discriminator now treats any stripped value beginning with { as inline JSON and parses it strictly, so malformed inline settings no longer fall through to the missing-file degradation path. The focused settings slice passes 10/10 and full tests/test_transport.py passes 210/210 locally, including after a clean merge with current main. This resolves my prior blocker.