fix(runner): send an MCP server the answer the person actually gave

[omni-resolve-agent]

Related issue

Closes #5272 — resolves OMNI-4778 (Linear).

Builds on #5273 by @Paldom — taken over because the fork branch was conflicting with main and needed review fixes that could not be pushed to the fork (App tokens can't push to fork branches). Commit e14b10625 is theirs, carried over with authorship preserved.

Summary

When an MCP server elicits input (e.g. ctx.elicit with an enum schema dev/staging/prod), the runner told the server the schema's first option, not the one the person actually picked.

• Root cause: the approval handler in runner/app.py dropped the verdict's content — pending_approvals was a dict[str, Future[bool]], so only the boolean survived, and mcp_manager._build_accept_content auto-filled the first enum value. proxy_mcp_manager dropped inputResponses content the same way.
• Fix: carry a full Verdict (approved + content) through pending_approvals; prefer the person's validated answer over the schema guess; fail closed (decline) on a non-conforming supplied answer instead of substituting one; make the no-content decline gate required-aware so optional-field consents still accept; and validate proxy MRTR inputResponses content against the requestedSchema via a shared validator (omnigent/tools/_elicitation_schema.py).

sequenceDiagram
    participant U as User (approval card)
    participant R as runner/app.py
    participant P as pending_approvals
    participant M as mcp_manager
    participant S as MCP server
    U->>R: approve + content {answer: "prod"}
    R->>P: resolve(Verdict(approved, content))  %% was: Future[bool] — content dropped
    P->>M: verdict
    M->>M: validate content against requestedSchema
    M->>S: ElicitResult(accept, {answer: "prod"})  %% was: first enum value "dev"

Loading

ELI5: the approval pipe only had room for "yes/no", so the actual answer fell on the floor and the code guessed the first option from the schema. The pipe now carries the answer too, checks it against the schema, and refuses (declines) rather than guessing when the answer doesn't fit.

Test Plan

Fail→pass proof (each facet fails on unfixed main, passes on this branch):

• E2E tests/e2e/test_mcp_elicitation_user_answer.py — user picks the 3rd enum option (prod); on unfixed main the tool output was elicit_answer:dev (FAIL), on this branch it is elicit_answer:prod (PASS). Re-verified green after merging latest main.
• Unit/integration tests/runner/test_mcp_elicitation_content.py (17 tests) — chosen option reaches the waiting caller; non-conforming answer declines (fail closed) instead of substituting; required-aware decline gate (test_an_unanswerable_schema_declines_rather_than_inventing, test_optional_fields_still_accept_without_an_answer); proxy MRTR inputResponses carries validated content and declines non-conforming browser content.
• Surrounding suites green: tests/runner/test_pending_approvals.py, tests/runner/test_runner_idle_active_work.py (40 tests total).

Independent review: a cross-vendor (codex) reviewer flagged 2 blocking issues (invalid supplied content fell back to a schema guess; decline gate ignored required) and 1 security issue (proxy MRTR path forwarded browser content unvalidated) — all fixed at the root with tests for each.

Validate the fix live

This fix runs in the runner/host process, so check out the PR — attaching a local runner to a UI preview would run an unfixed runner:

gh pr checkout <this PR>
omnigent claude -p 'Reproduce and validate a bug fix. Steps: configure an agent with a stdio MCP server whose deploy tool calls ctx.elicit with schema {type: object, properties: {answer: {type: string, enum: [dev, staging, prod]}}} (tests/tools/fixtures/elicitation_enum_mcp_server.py is exactly this); ask the agent to deploy; when the approval card renders the three option buttons, click the third option "prod". Before this fix, the MCP server received "dev" (the schema first enum value) regardless of the choice. Confirm the fix by checking the tool output contains elicit_answer:prod and not elicit_answer:dev. Also confirm: an answer outside the enum (POST content {answer: "production"} to the approval event) now yields a decline rather than a substituted answer, and a schema with only optional fields still accepts a bare approve. Report whether each behaves correctly.' --server ''

--server '' runs a local server from this same checkout, so the server and runner are the PR build.

Demo

• Visual demo attached below
• Non-visual evidence provided below or in Test Plan
• Not applicable — no behavioral change

After-fix recording (send "Please deploy." → approval card renders dev/staging/prod → click prod → tool output shows elicit_answer:prod, the user's actual choice): recordings/mcp-elicitation-answer/after-web.mp4, preserved in the resolve run's CI artifact bundle — this environment cannot attach media files directly to the PR.

Type of change

• Bug fix
• Feature
• UI / frontend change
• Refactor / chore
• Docs
• Test / CI
• Breaking change

Test coverage

• Unit tests added / updated
• Integration tests added / updated
• E2E tests added / updated
• Manual verification completed
• Existing tests cover this change
• Not applicable

Coverage notes

All facets are covered by automated tests that fail on unfixed main and pass here (see Test Plan).

Changelog

MCP elicitation now sends the server the option you actually picked, and declines answers that don't match the requested schema instead of substituting the first option

[github-actions]

UI Preview for this PR has been removed.

[omnigent-ci]

Polly AI Review

Summary of review scope

I read the entire diff and cross-checked it against the current source: every caller of pending_approvals.resolve / wait_for_user_approval / wait_for_user_verdict, the shared validate_content_against_schema helper, the pre-existing build_accept_content_from_schema fallback, and both elicitation code paths (inline mcp_manager._elicit and proxy proxy_mcp_manager.call_tool MRTR retry).

Blocking issues

None. The core fix is correct and complete:

• The Verdict dataclass threads content from the events endpoint through pending_approvals to both elicitation callers, replacing the lossy Future[bool]. resolve() correctly normalizes content to None on a refusal (content if approved else None), so declines/timeouts can never smuggle a stale answer.
• The precedence in _elicit is sound and ordered defensively: validated user answer → fail-closed decline on a supplied-but-nonconforming answer → schema auto-fill only when no answer was given → decline when a bare accept can't satisfy a required schema. The empty-dict case (verdict.content == {}) correctly falls through to auto-fill rather than declining, because _validated_content returns None for falsy content and the verdict.content guard is also falsy.
• The required-aware decline gate (_schema_requires_fields) correctly distinguishes bare-consent and all-optional schemas (which legally accept empty content) from schemas that mandate fields, preventing a regression that would have turned optional-field consents into declines.
• The proxy MRTR path (_input_response) now validates browser-supplied content against requestedSchema before forwarding, and reads the schema from the correct location (inputRequests[id].params.requestedSchema), matching the wire shape the Omnigent server sends.

Backward compatibility is preserved: wait_for_user_approval is retained as a thin .approved wrapper, resolve() keeps its (id, approved) positional contract with content defaulted, and the events handler guards isinstance(_elicit_content, dict) so malformed payloads degrade to the auto-fill fallback rather than raising.

Security vulnerabilities

None introduced — this change tightens an existing trust boundary. Content originating from the browser/events endpoint is now schema-validated in both the inline and proxy paths: keys must be declared properties (extra/undeclared keys → decline), values must match declared types and enum membership, and required fields must be present. Anything else fails closed with a decline rather than forwarding unvalidated fields or substituting a schema guess. The proxy path explicitly refuses to cross the wire with nonconforming content, which is the right call. No secrets, injection, or deserialization concerns in the diff.

Non-blocking notes

• Schema-less prompts with supplied content decline. In both paths, when a server sends no requestedSchema (or a non-dict one) but the verdict carries content, validate_content_against_schema returns None, and the content is None and verdict.content guard then declines. This is defensible (a schema-less surface shouldn't be collecting form fields), but it's a behavior worth being aware of: a valid bare-consent accept that happens to arrive with stray content becomes a decline rather than a bare accept. Not a bug for the flows exercised here, since schema-less prompts don't render form inputs.
• enum on an array property. _value_matches_property interprets a property-level enum as the set of allowed member values when the type is array. That's a reasonable best-effort, but JSON Schema conventionally places item constraints under items, not a property-level enum. Fine for the schemas MCP servers actually emit today; just noting it isn't a general JSON-Schema validator.
• The stale-sidebar-badge caveat (no response.elicitation_resolved on timeout/cancellation) is pre-existing and correctly left as a documented TODO rather than expanded here.

Overall assessment

A focused, well-reasoned fix for a real correctness bug (the server acting on an answer nobody chose). The design choice to fail closed — decline rather than substitute — on a nonconforming answer is the right one, and it doubles as a security hardening of the browser→server content boundary. Test coverage is excellent: an E2E fail→pass regression test with a dedicated fixture, plus unit/integration coverage of the chosen-option, bare-consent, decline, timeout, required-aware, enum/type-mismatch, and proxy MRTR cases. No visual demonstration is needed — this is an internal data-flow fix with no UI rendering change, and the E2E test provides the before/after proof. Recommend merge.

---

_{Automated review by Polly · workflow run}

[omni-resolve-agent]

Triage of the Polly review notes (all non-blocking; none require a code change):

1. Schema-less prompt + stray supplied content → decline — intentional fail-closed behavior, kept as-is. A schema-less elicitation renders no form inputs, so content arriving on it is anomalous (a malformed or forged event payload); declining is consistent with the fix's refuse-rather-than-guess design and never affects the bare-consent path (empty/absent content still yields a bare accept).
2. Property-level enum on an array type — acknowledged best-effort, kept as-is. MCP elicitation requestedSchema is spec-restricted to flat primitive properties, so a general JSON-Schema validator is out of scope; the array branch is a defensive extra, not a contract.
3. Stale sidebar badge on timeout/cancellation — pre-existing and documented as a TODO; deliberately not expanded in this bug-fix PR.

[omni-resolve-agent]

@bbqiu this fixes #5272 (OMNI-4778) — ready for your review.

• CI: green on everything this diff touches. The one red check, Pytest (misc), is pre-existing on main — tests/test_codex_native.py::test_codex_discover_thread_login_required_* fails with TypeError: _codex_discover_thread_and_forward() missing 1 required keyword-only argument: 'workspace' on a clean origin/main checkout (verified locally at 66a28e440); this PR touches no codex files.
• Polly review: clean — no blocking or security findings; the three non-blocking notes are triaged in the comment above.
• Fail→pass proof: tests/e2e/test_mcp_elicitation_user_answer.py fails on unfixed main (elicit_answer:dev) and passes here (elicit_answer:prod), plus 17 targeted runner tests.
• Try it live (runner-side fix, so use the PR checkout — a preview-attached local runner would be unfixed):

  gh pr checkout 5957
  omnigent claude -p 'Reproduce and validate a bug fix. Steps: configure an agent with a stdio MCP server whose deploy tool calls ctx.elicit with schema {type: object, properties: {answer: {type: string, enum: [dev, staging, prod]}}} (tests/tools/fixtures/elicitation_enum_mcp_server.py is exactly this); ask the agent to deploy; when the approval card renders the three option buttons, click the third option "prod". Before this fix, the MCP server received "dev" regardless of the choice. Confirm the tool output contains elicit_answer:prod and not elicit_answer:dev. Also confirm: an answer outside the enum yields a decline rather than a substituted answer, and a schema with only optional fields still accepts a bare approve.' --server ''
  

  UI preview (server side only): https://omnigent-ui-preview-pr-5957-3272836215725701.aws.databricksapps.com

[omnigent-ci]

Polly AI Review

Blocking issues

None. The core fix is sound: pending_approvals now carries a full Verdict(approved, content), the resolver normalizes content to None on refusal, and _elicit prefers the person's validated answer, falls back to the schema auto-fill only when no content was collected, and fails closed (declines) on a non-conforming supplied answer. The regression is pinned end-to-end (through the real HTTP handler and the real elicitation callback), and the required-aware decline gate is covered on both the "required" and "all-optional" sides. I verified the changed call sites (app.py approval handler, both wait_for_user_* consumers, proxy MRTR) against the surrounding source and they are consistent.

Security vulnerabilities

None introduced — this PR strengthens a trust boundary. Both the inline (_validated_content) and proxy (_input_response) paths now run browser-supplied content through validate_content_against_schema, which rejects undeclared keys, wrong types, out-of-enum values, and missing required fields before anything crosses to the MCP server. The proxy path previously forwarded only accept/decline; it now validates any content it forwards and declines rather than smuggling unvalidated fields. bool/int disambiguation in _value_matches_property is handled explicitly.

Non-blocking notes

• Proxy vs. inline asymmetry on required-field, no-content accepts. The inline _elicit path adds a _schema_requires_fields gate: a bare accept (no content) against a schema with required fields declines. _input_response in proxy_mcp_manager.py has no equivalent gate — a bare approve verdict against a required-field schema returns {"action": "accept"} with no content, which the server's own schema will likely reject (and the retry loop then hits "Approval loop exceeded"). This is not a regression (the proxy never auto-filled or gated before), but the two paths now diverge on the same input; worth aligning for consistency.
• validate_content_against_schema collapses two distinct cases to None (empty content vs. non-conforming content). Callers disambiguate correctly via the separate verdict.content truthiness check, but the dual meaning is a subtle contract — a short note at the call sites (already partially present) is the only safeguard against a future caller misreading it.

Approach

Sound and consistent with existing patterns. Threading a small frozen Verdict dataclass through the existing registry (rather than widening the Future's payload ad hoc) is the right minimal shape, and keeping wait_for_user_approval as a thin consent-only wrapper over wait_for_user_verdict preserves every existing yes/no caller without churn. Centralizing validation in the shared omnigent/tools/_elicitation_schema.py so both the inline and proxy paths use one validator is the correct DRY choice for a trust-boundary check. No materially simpler alternative stands out.

Summary

A well-scoped, well-tested bug fix that closes a real correctness defect (the MCP server was told the schema's first enum option instead of the user's choice) and hardens the elicitation trust boundary on both the inline and proxy paths. Content flows through a shared schema validator, fails closed on non-conforming input, and the required-aware decline gate avoids inverting optional-field consents. Fail→pass proof is provided at unit, integration, and E2E levels. The only follow-up worth considering is aligning the proxy MRTR path's handling of bare accepts against required-field schemas with the inline path's decline gate. Recommend merge after the author considers that non-blocking point.

---

_{Automated review by Polly · workflow run}

[daniellok-db]

Polly review triage (head 2ffde13e8) — both non-blocking notes from the previous review addressed at the root:

1. Proxy vs. inline asymmetry on required-field bare accepts. proxy_mcp_manager._input_response now declines a content-less accept when the requestedSchema marks fields required, matching mcp_manager._elicit's required-aware gate (a bare accept there is malformed and spins the MRTR retry loop / "Approval loop exceeded"). The required-fields predicate moved into the shared omnigent/tools/_elicitation_schema.py as schema_requires_fields() so the inline and proxy paths share one implementation; mcp_manager._schema_requires_fields now delegates to it. New unit test test_proxy_input_response_declines_a_bare_accept_when_fields_are_required (verified fail→pass: accepts without the gate, declines with it). The all-optional bare accept still accepts (test_proxy_input_response_bare_accept_and_decline).

2. Dual meaning of validate_content_against_schema returning None. The docstring now states explicitly that None covers both "no content" and "non-conforming content", and that callers disambiguate via the supplied content's own truthiness — fail closed when an answer was given but rejected, fall back when none was given.

Also merged latest main (the branch was ~150 commits behind). That resolves the earlier Pytest (misc) red, which was pure merge drift — tests/test_codex_native.py was missing the workspace= kwarg that main had already added to _codex_discover_thread_and_forward — and is unrelated to this fix. CI + Polly re-triggered on the new head.

[daniellok-db]

lg, tested locally

[omnigent-ci]

Polly AI Review

Blocking issues

None that hold up under verification. Two candidates were raised and checked against the code:

• Bare-accept on a required enum still auto-fills the first value. In mcp_manager._elicit, the order is _validated_content → (decline if supplied-but-nonconforming) → _build_accept_content → required-field decline gate. Because _build_accept_content fills {"answer": "dev"} before the _schema_requires_fields gate runs, a content-less accept on a required enum never reaches the decline. This looks like it re-creates the bug, but it's the documented REPL / binary-approve fallback (pinned by test_a_bare_approval_still_falls_back_to_the_schema): here nobody chose, so guessing on a field-less surface is intended, not the "user chose X, server got Y" defect. It diverges from the proxy path (which declines) — a real inconsistency, but not a correctness bug. Non-blocking.
• Empty {} content treated as absent. validate_content_against_schema returns None for {} (if not content). For an all-optional schema this yields accept with no content instead of accept {}. An empty accept is valid for an all-optional schema, so no server contract is broken. Non-blocking.

The core fix is correct: Verdict(approved, content) threads cleanly through resolve → wait_for_user_verdict → both managers; the person's validated answer is preferred; declines/timeouts are normalized to content=None at the registry so form data can't leak past a refusal; and the wait_for_user_approval wrapper is retained for compat (though it now has no production callers — see below).

Security vulnerabilities

The new browser→server content path has an incomplete validator (should-fix, bounded). Before this PR the proxy path never forwarded content (bare accept/decline); it now forwards browser-supplied content, guarded only by validate_content_against_schema. That validator checks type, enum membership, and oneOf[].const, but I confirmed by direct execution that it silently passes several non-conforming shapes:

• anyOf: [{type:string},{type:null}] (i.e. any Python str | None optional field) has no top-level type, so all type checks are skipped — {"note": 123} validates. This is the most reachable gap, since optional-nullable fields are extremely common.
• Numeric/string constraints are ignored: 999 passes maximum: 100; "ab" passes minLength: 5.
• Bare property-level const is not enforced ({"answer":"dev"} passes const: "prod").
• type: array validates only as list[str] — items, items.enum, minItems/maxItems are unchecked, so ["prod","smuggled"] passes an array restricted to dev/prod (largely theoretical, since MCP's restricted elicitation schema doesn't emit arrays).

Impact is bounded, not a bypass: enum membership and type confusion on typed fields are caught, non-primitive values are rejected, and the MCP server/SDK re-validates content and refuses malformed values. So the residual effect is a graceful decline being replaced by a hard server-side rejection, plus a weaker-than-advertised runner-side boundary — not injection/RCE/auth bypass. Given the PR explicitly positions this validator as the trust-boundary check ("not forwarded across the trust boundary"), tightening it to at least handle anyOf/nullable type unions and numeric/length bounds (or explicitly documenting the server as the authoritative validator) is worth doing before the claim stands. No secret exposure or unsafe-deserialization issues found.

Non-blocking notes

1. Proxy declines a conforming answer when the server omits requestedSchema. _input_response derives schema from input_request["params"]["requestedSchema"]; if absent while verdict.content is truthy, validate_content_against_schema(content, None) → None → the content is None and verdict.content branch fires → {"action":"decline"}. Correctness now depends on the Omnigent server always echoing requestedSchema into inputRequests[eid].params. Fail-closed is the safe direction, but a one-line confirmation the server always populates it would prevent a legitimate answered accept silently becoming a decline.
2. wait_for_user_approval has zero production callers now — both managers moved to wait_for_user_verdict. The wrapper + its tests remain (good for compat), but the prose in orchestration.py:1874/2006/6892 still references a "wait_for_user_approval park." Doc drift; flag so the keep is deliberate.
3. Typeless enum + boolean: an enum with no type accepts True for an integer-valued enum (True == 1 in Python). Unreachable via the web form (sends strings) and via typed integer enums (bool rejected pre-enum). Cosmetic.
4. Test coverage misses the two edge cases above: no test pins preserved {} content for all-optional inline accepts, and content-less required-enum behavior is only exercised on the proxy path, not the inline auto-fill path.

Approach

Sound and consistent with the repo. Threading a Verdict dataclass through pending_approvals and factoring validation into a shared omnigent/tools/_elicitation_schema.py helper reused by both the inline and proxy paths is the right shape — it keeps the fix in one place and lets the proxy MRTR path reuse identical validation. The only structural nit is the inline-vs-proxy divergence on bare-accept-with-required-fields; unifying those two gates (both decline, or both fall back) would remove a genuine behavioral inconsistency.

Summary

A well-scoped, well-tested fix for a real user-facing defect: the MCP server now receives the answer the person actually picked rather than the schema's first enum value, with sensible fail-closed handling. The fail→pass E2E test plus 33 passing unit/integration tests give strong confidence in the primary behavior. No blocking correctness bugs survive verification. The one thing worth addressing before merge is the completeness of the schema validator that now guards a new browser→server content path — it passes wrong-typed nullable/optional values, unchecked numeric/length bounds, and property-level const; the server re-validates so impact is bounded, but the PR's own trust-boundary claim warrants tightening anyOf/nullable and bound handling (or documenting the server as authoritative). Recommend addressing the validator gap and the two non-blocking proxy/consistency notes; otherwise ship-ready.That was the codex result I already collected and incorporated. Both cross-vendor reviews are complete and the consolidated review has been posted. Nothing further is pending.

---

_{Automated review by Polly · workflow run}

[omni-resolve-agent]

🤖 Otto merge started

Otto is reviewing this approved PR, addressing actionable review feedback and CI failures, and resolving merge conflicts if needed. It will merge only after current approval, CI, Polly, review threads, and mergeability all pass.

[github-actions]

🏷️ Doc impact: no-doc-update

Internal bugfix to MCP elicitation content plumbing (carrying the user's answer through the verdict registry and validating it against the requestedSchema); no user-facing surface, integration, or documented default changed.

_{Auto-classified on merge. Set the label manually before merging to override. · run}

[omnigent-ci]

Polly AI Review

Review: fix(runner): send an MCP server the answer the person actually gave

The fix is well-targeted and the root-cause analysis is correct: pending_approvals was a Future[bool], so the user's answer was dropped and _build_accept_content invented the first enum value. Carrying a full Verdict(approved, content), validating it against requestedSchema, and failing closed on non-conforming content is the right shape. The wait_for_user_approval wrapper preserves backward compatibility, the E2E fixture reproduces the exact defect, and undeclared top-level keys are correctly rejected by validate_content_against_schema. The following are worth addressing.

Blocking issues

1. Empty submitted content {} is conflated with "no content", re-inventing an unchosen value — mcp_manager._elicit (and validate_content_against_schema, _elicitation_schema.py).

validate_content_against_schema starts with if not content: return None, so an explicitly-submitted empty object {} returns None — indistinguishable from a verdict that carried no content at all. In _elicit the decline guard is truthiness-based:

content = _validated_content(verdict.content, params)   # {} -> None
if content is None and verdict.content:                 # {} is falsy -> skipped
    return ElicitResult(action="decline")
if content is None:
    content = _build_accept_content(params)             # invents enum[0] = "dev"

app.py sets verdict.content = {} (an empty dict passes isinstance(_elicit_content, dict)), so a user who accepts an enum prompt without a selection has {} autofilled to the schema's first option — the exact "server told a value nobody chose" bug this PR fixes, and it slips past even the required-aware gate because content is no longer None after autofill. Distinguish {} (explicit, conforming for an optional schema; should not autofill) from None (no content collected; autofill fallback intended). This depends on whether the approval frontend ever emits content: {} for a rendered-field card — please verify that path; if it can, this reintroduces the target defect.

Security vulnerabilities

None that constitute a bypass. Note (non-blocking) that the PR description claims proxy inputResponses content that doesn't fit ("wrong types, values outside an enum") "is not forwarded across the trust boundary." The validator is incomplete relative to full JSON Schema — it ignores pattern, exclusiveMinimum/exclusiveMaximum, multipleOf, does not validate non-const oneOf branches, and treats unknown declared types as matching (_matches_declared_type returns True). Such values pass the runner's check and are forwarded. This is a defense-in-depth gap, not an exploitable hole, because the MCP server that emitted the schema remains the authoritative validator and rejects non-conforming content (the MRTR loop then errors). Consider failing closed on unrecognized/unsupported constraint keywords so the claim matches the implementation.

Non-blocking notes

• Proxy missing-schema path accepts a bare approval — proxy_mcp_manager._input_response. When requestedSchema is absent/malformed and verdict.content is empty, the function returns {"action": "accept"} with no content (both fail-closed guards are skipped: schema_requires_fields(None) is False). The comment asserts the Omnigent server "always populates params.requestedSchema," so this is largely theoretical; if the schema were genuinely required, the server rejects and the retry loop returns "Approval loop exceeded" — a handled failure, not corruption. Fine to leave, but a schema is None → decline guard would be more robust.
• build_accept_content_from_schema declines when any optional free-form field can't be synthesized — a schema with required approved: boolean + optional note: string|null returns None (fails on note), so both paths decline even though {"approved": true} conforms. This is pre-existing conservative behavior now reached from the proxy path too; worth a follow-up but not introduced by this diff.
• _matches_declared_type rejects 1.0 for "type": "integer" though JSON Schema treats an integer-valued float as an integer — a narrow false-decline edge.

Approach

Sound and consistent with the repo: reusing the shared omnigent/tools/_elicitation_schema.py validator across the inline (mcp_manager) and proxy (proxy_mcp_manager) paths avoids divergence, and the wait_for_user_verdict/wait_for_user_approval split keeps consent-only callers unchanged. No materially simpler alternative — the complexity lives in schema validation, which is inherent to the goal. One consolidation option: fold the {}-vs-None distinction into the validator (return a sentinel or accept {} for schemas with no required fields) so both call sites get it for free rather than each re-deriving it from verdict.content truthiness.

Summary

A correct, focused fix for a real bug, backed by a genuine fail→pass E2E test and good fail-closed instincts. The one issue I'd resolve before merge is the {}-vs-None conflation in _elicit, which can silently re-invent an unchosen enum value for an empty-but-present submission — verify whether the approval UI can emit content: {} and, if so, distinguish it from absent content. The validator-completeness and missing-schema items are reasonable hardening follow-ups rather than merge blockers.

---

_{Automated review by Polly · workflow run}

[omni-resolve-agent]

Otto post-merge note

This PR was merged at head 73462ec417866f6658bd7c980d3337086fe8aa01, which includes the fix for the E2E shard-2 regression (proxy bare-accept auto-fill, 73462ec41) plus the validator tightening Polly requested (anyOf/nullable unions, numeric/length bounds, property-level const, array items enum/bounds).

Polly's review of 73462ec41 landed a few minutes after the merge and flagged one blocking issue that is therefore present in main: an explicitly submitted empty content {} is conflated with "no content collected", so accepting an all-optional enum prompt without a selection can still auto-fill the schema's first value — the same class of defect this PR fixed. I verified the web form (ElicitationSchemaForm.toContent) omits blank fields and can emit {}.

A fix with tests is ready on this branch as commit bc9974024004fac4bf9c70425c4504c829bf0f66 (branch fix/mcp-elicitation-user-answer was re-created by that push): validate_content_against_schema now keeps {} when the schema requires no fields and rejects it when fields are required, and both elicitation paths gate the fail-closed decline on content is not None. Validated locally: 81 targeted runner/REPL unit tests plus tests/e2e/test_mcp_elicitation_user_answer.py and the two REPL approval E2E tests all pass.

Please open a follow-up PR from that commit (or cherry-pick it) — this PR is already merged, so Otto is not opening a new PR on its own.