fix(tui): stop fleet readonly schema probe from auto-vivifying null enums

[gaord]

Summary

Fix project_readonly_evidence_schema: it probed for an action enum with IndexMut, which auto-vivifies missing keys as null. A read-only Fleet worker projecting a lowercase bash tool then carried properties.action = {"enum": null}, which strict OpenAI-compatible validators reject. Probe with non-mutating get_mut instead.

Testing

• cargo fmt --all -- --check
• cargo test -p codewhale-tui --lib fleet_readonly_reviewer_wire_catalog_carries_no_null_schema_fields

Note: main currently has 5 pre-existing nonminimal_bool clippy errors (tools/subagent, tui/ui/apply, tui/views/fleet_roster, tui/phase_strip), unrelated to this change.

---

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[Hmbown]

Reviewed at 8104a75b0. The bug is real, the fix is correct, and I reproduced both states locally. Approving in substance — two small reuse notes and a DCO trailer below.

What I verified

I checked the head out into a detached worktree, reverted only crates/tui/src/tools/registry.rs to its parent (8104a75b0^), and kept your new test. It fails exactly as you describe:

tool bash schema carries null at ["$.properties.action.enum"]:
{"additionalProperties":false,"properties":{"action":{"enum":null},"command":{...},...}}

Restoring the fix: 48 passed; 0 failed for tools::registry
(RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib -- tools::registry).

Two things worth recording because they are not obvious from the diff:

1. The auto-vivification is two levels deep, not one. schema["properties"]["action"] inserts Null, and then ["enum"] on that Null converts it into an object before inserting — which is why the artifact is {"enum": null} and not a bare null.
2. Neither schema_sanitize::sanitize nor schema_canonicalize::canonicalize_schema (registry.rs:270-271) strips it downstream. The failure output above is post-sanitize, post-canonicalize. So the guard genuinely has to live at the probe — there is no net underneath it.

Nits

1. pointer_mut is already the house idiom for this exact probe — crates/tui/src/tools/subagent/mod.rs:14869 and :14907:

let Some(actions) = tool
    .input_schema
    .pointer_mut("/properties/action/enum")
    .and_then(serde_json::Value::as_array_mut)
else { continue; };

and the comment right above it (mod.rs:14863-14867) describes this same bug, down to the lowercase-bash shape:

Indexing ["properties"]["action"]["enum"] mutably would fabricate an "action": {"enum": null} property on schemas that have no action discriminator (the lowercase bash command/timeout shape) — a phantom node that fails Moonshot MFJS validation. Only shape enums that already exist.

So this is the same known defect in a second location, and the established one-line fix collapses your three-step ladder to:

let Some(actions) = schema
    .pointer_mut("/properties/action/enum")
    .and_then(Value::as_array_mut)
else { return; };

Same fail-closed behavior, non-mutating, and it makes the two sites greppable as one pattern. Your long comment is worth keeping either way — it is better than the one in subagent.

2. The Run arm two lines above still auto-vivifies — crates/tui/src/tools/registry.rs:508:

if let Some(properties) = schema["properties"].as_object_mut() {

If a Run-family schema ever lands without a top-level properties, this writes "properties": null into the wire schema and then returns — the identical failure class one match arm earlier. It is latent today (Run always has properties), but since this PR is specifically about closing that hole, schema.get_mut("properties").and_then(Value::as_object_mut) there would close it for good. Your new test would not currently catch it, because Run is filtered out of the reviewer catalog unless verification == Bounded.

3. DCO — 8104a75b0 carries no Signed-off-by: trailer. The Check Signed-off-by job is advisory (.github/workflows/dco.yml exits 0 either way), so it is green, but CONTRIBUTING asks for it. git commit --amend -s && git push --force-with-lease when convenient.

Unrelated to this change: the Test (windows-latest) failure on this run does not touch tools::registry.

Nice catch, and thank you for the regression test that walks the whole projected catalog for nulls rather than asserting on bash alone — that generalizes to the next tool that grows an action enum.