feat(derive): accept a value the OS accepts and UTF-8 does not

[jdx]

Reporting a non-UTF-8 value was the safe half of the fix. It is not the whole one:
/tmp/\xff is a filename the operating system accepts, and a CLI that cannot receive
one cannot open the file. A PathBuf or OsString field now takes the bytes exactly.

usage_argv::os_string_from_bytes is the reverse of as_encoded_bytes. On Unix it is
the safe OsString::from_vec — an OsString there is an arbitrary byte sequence, so
the conversion is total and no unsafe arises. Only Windows needs
from_encoded_bytes_unchecked, because WTF-8 makes the conversion partial.

That call rests on an invariant, now written where it can be checked: every sub-slice
the parser produces is cut at an ASCII byte — after -, after --, at =, and between
the letters of a short-flag cluster — and an ASCII byte never occurs inside a multi-byte
sequence. The one way to break it is a non-ASCII Flag::shorts entry, which would let a
cluster split mid-character. The derive already refused that; the rule is now documented
as load-bearing on both sides and has the test it was missing.

The crate goes from forbid(unsafe_code) to deny, so the single audited exception has
to name itself rather than the crate pretending it has none. Parsing still contains no
unsafe at all.

Byte-exactness is cheaper than the text path, not dearer: a PathBuf field costs 567
instructions per parse against a String field's 660, because it skips the UTF-8
validation pass, and both allocate once. The gate fixture cannot show this — a spec
carries no Rust types, so every shadow field is a String — so it is measured directly.

A String field still reports rather than substitutes, which is the right answer for a
type that cannot hold those bytes.

CI gains a Windows compile check, since every test here is #[cfg(unix)] and nothing in
the pipeline was building the branch that carries the unsafe.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

---

Note

Medium Risk
Touches argv-to-field conversion for path types and platform-specific behavior on Windows; Unix behavior is well-tested but the Windows path is compile-checked only, not exercised in tests.

Overview
PathBuf and OsString fields now keep argv bytes instead of requiring UTF-8, so paths like /tmp/\xff parse as the OS sees them rather than being rejected or mangled. The derive routes those types through new usage_argv::os_string_from_bytes: lossless OsString::from_vec on Unix, and on Windows a safe UTF-8 round-trip (invalid bytes become InvalidValue) rather than from_encoded_bytes_unchecked, so the crate stays forbid(unsafe_code).

Docs tie ASCII-only short flags to parser slicing (clusters and = cuts), with derive validation and a test reinforcing that. Unix conformance tests cover detached, attached, short, repeated, and OsString forms; String still errors on non-UTF-8. CI adds a x86_64-pc-windows-msvc cargo check for the non-Unix branch. PLAN.md marks the milestone done.

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

Cost

A PathBuf field is now cheaper than the same field as a String, because it skips the
UTF-8 validation pass:

field type	instructions per parse	allocations per parse
Option<PathBuf>	567	1
Option<String>	660	1

The gate fixture cannot show this and was not used for it: a spec carries no Rust types, so
every field in the generated shadow is a String and the shadow's numbers are unchanged
(40,472 against 40,484 on the parent — the same code path, and the difference is noise).
Measured instead on a two-field probe, differencing PARSE_N=0 against PARSE_N=100 runs of
the same binary and dividing.

Why this is sound

On Unix there is no unsafe: an OsString is an arbitrary byte sequence, so the
conversion is the safe OsString::from_vec and no invariant is required. This is worth
stating plainly because it means the approval was only needed for one platform.

On Windows the encoding is WTF-8, not every byte sequence is valid, and only
from_encoded_bytes_unchecked will take one. That rests on an invariant of the crate:

Every sub-slice the parser produces is cut at an ASCII byte.

A token is split after -, after --, at =, and between the letters of a short-flag
cluster. All four are ASCII, and an ASCII byte never occurs inside a multi-byte WTF-8 or UTF-8
sequence, so no cut can land mid-character. Values passed through whole are trivially fine.

The one way to break it is a non-ASCII Flag::shorts entry, which would let a cluster split
mid-character. The derive already rejected that — for ergonomic reasons — so the rule was
already true; what this PR adds is that it is now documented as load-bearing on both the
field and the check, and has the test it was missing.

Verification

• Every route a value can take keeps its bytes: detached (--out <bad>), attached
  (--out=<bad>, cut at the =), short (-o<bad>), short-with-equals (-o=<bad>),
  repeated into a Vec<PathBuf>, and a bare OsString field. The cluster cases are the ones
  the soundness argument leans on hardest, which is why they are enumerated rather than
  sampled.
• Mutation-checked: with the conversion switched to from_utf8_lossy, both byte-exactness
  tests fail.
• Option<Vec<PathBuf>> still tells "never given" from "given nothing" — that decision is
  made before the conversion and had to survive the new path.
• A String field still reports rather than substitutes, since it cannot hold those bytes.
• The non-ASCII short rejection now has a test, and the unsafe branch is compile-checked
  for x86_64-pc-windows-msvc.

One gap worth naming

Every test here is #[cfg(unix)], and CI had no Windows job — so the branch carrying the
unsafe was the one thing in the pipeline nothing built. This adds a Windows compile check
to test.yml. A compile check is not coverage: the Windows conversion is still unexercised
by any test, and reaching it needs an unpaired surrogate in argv. If that matters enough to
you, the next step would be a Windows test job rather than a check.

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

Summary by CodeRabbit

• New Features

  • Added support for preserving non-UTF-8 command-line values in path and operating-system string inputs on compatible platforms.
  • Added short-form output support, repeated path inputs, raw values, and optional repeated exclusions.
  • Added clear errors when values cannot be converted on supported platforms.

• Bug Fixes

  • Fixed path argument handling so raw bytes are preserved instead of being altered.
  • Improved validation and error reporting for non-ASCII short flags.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds safe conversion from argument bytes to OsString, preserves non-UTF-8 values for typed path and OS-string fields, expands conformance coverage, documents ASCII short flags, and adds a Windows compilation check.

Changes

Non-UTF-8 OS value preservation

Layer / File(s)	Summary
OS string conversion contract argv/src/lib.rs, PLAN.md	The public os_string_from_bytes function preserves bytes on Unix and validates UTF-8 on other platforms. Documentation and the implementation plan describe this behavior.
Typed field conversion derive/src/codegen.rs, derive/src/model.rs	Generated parsing preserves non-UTF-8 values for required, optional, repeated, and optional-collection PathBuf and OsString fields. Non-ASCII short flags are rejected.
Conformance and platform validation conformance/tests/typed.rs, .github/workflows/test.yml	Tests cover byte preservation across supported argument forms, retain String rejection checks, and compile the package for Windows with all features.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Mergeability Score: ⚪ Minimal · up to 50c28

The PR adds byte-exact OS argument handling with bounded platform-specific behavior and documented tests; no actionable merge-blocking risk remains beyond a minor documentation-formatting cleanup.

Possibly related PRs

• jdx/usage#798: Introduced the argv/src/lib.rs byte-preserving parsing foundation extended by this PR.
• jdx/usage#833: Added related typed-field conversion changes in derive/src/codegen.rs.
• jdx/usage#841: Established the raw-byte preservation pipeline extended by this PR.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant Parser
  participant field_final
  participant os_string_from_bytes
  participant TypedField
  CLI->>Parser: provide argument bytes
  Parser->>field_final: pass parsed bytes
  field_final->>os_string_from_bytes: convert PathBuf or OsString bytes
  os_string_from_bytes-->>field_final: return OS value or InvalidValue
  field_final-->>TypedField: assign required, optional, or repeated value

Loading

Poem

A rabbit hops through bytes unseen,
Paths stay whole, both rough and clean.
Short flags keep to ASCII’s lane,
Windows checks the build again.
Non-UTF-8 values now run free—
Hoppy parsing, safely! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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 summarizes the main change: accepting OS-valid values that are not valid UTF-8.

---

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

[github-actions]

Instruction counts

The comparison never ran — an earlier step failed.

_{426f1bc06c4c vs `` · measured on the runner, not pushed to the history.}

[greptile-apps]

Greptile Summary

The PR safely completes the prior fix by preserving arbitrary argv bytes for PathBuf and OsString on Unix while rejecting non-UTF-8 encoded values on Windows.

• Adds os_string_from_bytes with a safe Unix conversion and validated non-Unix fallback.
• Routes derived path and OS-string fields through byte-preserving conversion.
• Adds Unix conformance coverage and a Windows cross-compilation check.
• Documents and tests the ASCII short-flag invariant.

Confidence Score: 5/5

The PR appears safe to merge.

The previously reported unchecked Windows conversion has been removed, and no blocking failure remains.

Important Files Changed

Filename	Overview
argv/src/lib.rs	Replaces the previously unsafe Windows conversion with a safe, explicitly fallible UTF-8 path while preserving arbitrary bytes on Unix.
derive/src/codegen.rs	Generates byte-preserving finalization for standard PathBuf and OsString field shapes and maps conversion failures to InvalidValue.
derive/src/model.rs	Documents the existing ASCII short-name invariant and adds focused validation coverage.
conformance/tests/typed.rs	Covers byte-exact Unix conversion across attached, detached, repeated, optional-collection, path, and OS-string inputs.
.github/workflows/test.yml	Adds a Windows target compile check for the platform-specific conversion branch.

_{Reviews (6): Last reviewed commit: "fix(argv): make the byte conversion soun..." | Re-trigger Greptile}

[jdx]

Addressed in the latest push.

The P1 was correct and the fix was to remove the `unsafe`, not to justify it better. A safe function taking a `Vec` cannot enforce a "these came from `as_encoded_bytes`" precondition, so no amount of documentation made it sound.

On Unix an `OsString` is an arbitrary byte sequence, so `OsString::from_vec` is exact and safe — that is the case that matters, since non-UTF-8 filenames are ordinary there. On Windows the bytes go through UTF-8 and one that will not convert comes back in the `Err` to be reported. The crate `forbid(unsafe_code)`s again. Byte-exactness is still cheaper than the text path: 553 instructions per parse against 674.

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

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

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

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

[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 `@PLAN.md`:
- Around line 140-148: Dedent the paragraph beginning “And with no unsafe
anywhere.” so it is rendered as prose within the surrounding checklist item
rather than as an indented code block, while preserving its text and paragraph
structure.

🪄 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: a2a656a6-5bab-46dc-a2ab-33cf7a23d04c

📥 Commits

Reviewing files that changed from the base of the PR and between cca5793 and 50c28ee.

📒 Files selected for processing (6)

• .github/workflows/test.yml
• PLAN.md
• argv/src/lib.rs
• conformance/tests/typed.rs
• derive/src/codegen.rs
• derive/src/model.rs

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Render this text as prose.

Line 140 starts an indented code block after the blank line. Markdownlint reports MD046. Dedent this paragraph so it remains part of the checklist item.

Proposed fix

-      **And with no `unsafe` anywhere.** On Unix an `OsString` is an arbitrary byte sequence,
-          so this is the safe `OsString::from_vec` and every byte survives — which is the case that
-          matters, since non-UTF-8 filenames are ordinary there. Windows was going to need
-          `from_encoded_bytes_unchecked`, and jdx approved that, but a *safe* function taking a
-          `Vec<u8>` cannot enforce its precondition: there is no way to know the bytes came from
-          `as_encoded_bytes` rather than from anywhere else, and a safe function whose precondition
-          a caller can violate is unsound however carefully today's callers behave. Greptile flagged
-          exactly that on `#844`. So Windows goes through UTF-8 and reports what will not convert,
-          which gives up only an unpaired-surrogate argument there.
+  **And with no `unsafe` anywhere.** On Unix an `OsString` is an arbitrary byte sequence,
+  so this is the safe `OsString::from_vec` and every byte survives — which is the case that
+  matters, since non-UTF-8 filenames are ordinary there. Windows was going to need
+  `from_encoded_bytes_unchecked`, and jdx approved that, but a *safe* function taking a
+  `Vec<u8>` cannot enforce its precondition: there is no way to know the bytes came from
+  `as_encoded_bytes` rather than from anywhere else, and a safe function whose precondition
+  a caller can violate is unsound however carefully today's callers behave. Greptile flagged
+  exactly that on `#844`. So Windows goes through UTF-8 and reports what will not convert,
+  which gives up only an unpaired-surrogate argument there.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	**And with no `unsafe` anywhere.** On Unix an `OsString` is an arbitrary byte sequence,
	so this is the safe `OsString::from_vec` and every byte survives — which is the case that
	matters, since non-UTF-8 filenames are ordinary there. Windows was going to need
	`from_encoded_bytes_unchecked`, and jdx approved that, but a *safe* function taking a
	`Vec<u8>` cannot enforce its precondition: there is no way to know the bytes came from
	`as_encoded_bytes` rather than from anywhere else, and a safe function whose precondition
	a caller can violate is unsound however carefully today's callers behave. Greptile flagged
	exactly that on #844. So Windows goes through UTF-8 and reports what will not convert,
	which gives up only an unpaired-surrogate argument there.
	**And with no `unsafe` anywhere.** On Unix an `OsString` is an arbitrary byte sequence,
	so this is the safe `OsString::from_vec` and every byte survives — which is the case that
	matters, since non-UTF-8 filenames are ordinary there. Windows was going to need
	`from_encoded_bytes_unchecked`, and jdx approved that, but a *safe* function taking a
	`Vec<u8>` cannot enforce its precondition: there is no way to know the bytes came from
	`as_encoded_bytes` rather than from anywhere else, and a safe function whose precondition
	a caller can violate is unsound however carefully today's callers behave. Greptile flagged
	exactly that on #844. So Windows goes through UTF-8 and reports what will not convert,
	which gives up only an unpaired-surrogate argument there.

🧰 Tools

🪛 markdownlint-cli2 (0.23.2)

[warning] 140-140: Code block style
Expected: fenced; Actual: indented

(MD046, code-block-style)

🤖 Prompt for 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.

In `@PLAN.md` around lines 140 - 148, Dedent the paragraph beginning “And with no
unsafe anywhere.” so it is rendered as prose within the surrounding checklist
item rather than as an indented code block, while preserving its text and
paragraph structure.

Source: Linters/SAST tools