Skip to content

Loud tool-argument validation: eliminate silent discard/degradation of LLM tool arguments#1398

Merged
Aaronontheweb merged 8 commits into
netclaw-dev:devfrom
Aaronontheweb:investigate/agent-read-write-build-loop
Jun 12, 2026
Merged

Loud tool-argument validation: eliminate silent discard/degradation of LLM tool arguments#1398
Aaronontheweb merged 8 commits into
netclaw-dev:devfrom
Aaronontheweb:investigate/agent-read-write-build-loop

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Why

LLM-supplied tool arguments could be silently discarded, coerced, or overridden with no signal to the agent. Origin: production session D0AC6CKBK5K_1781115410_840529, where the agent passed "TimeoutSeconds":"1200" to extend a shell timeout, but Netclaw only recognizes the meta key _timeout_seconds. The key was silently dropped, the timeout fell back to a default, the agent's delegation call was killed mid-flight, and the agent — believing it had set a generous timeout — spun in a stuck loop.

A follow-up audit (SILENT_FALLBACK_AUDIT.md) found 17 sites across the tool-call surface sharing three mechanisms. The config surface already enforces this discipline (additionalProperties:false + ConfigSchemaDoctorCheck); the live tool-argument surface had no equivalent. This PR closes that gap.

What changes

  • Unknown argument keys reject before execution with a recoverable, self-describing error and a "did you mean _timeout_seconds?" suggestion. Fuzzy matching generates the suggestion text only — it never accepts a near-miss key (the LLM re-issues explicitly).
  • Present-but-invalid values reject instead of coercing to 0/false/null. Absent optional params keep their documented defaults.
  • Malformed args JSON from the provider boundary rejects per call id instead of dispatching the tool with null arguments.
  • Every honored-but-overridden value is surfaced to the model (timeout clamp/floor) as a notice appended after output bounding.
  • web_fetch validates Format (no silent raw fallback) and surfaces 5 MB truncation; list_webhooks honors its previously-dead Filter parameter.
  • All pre-dispatch validation is consolidated at the shared executor seam so the sub-agent path gets it too; rejections are audited Allowed=false.
  • netclaw-operations system skill updated (timeout-clamp + _background guidance, strict argument naming).

Behavior change: calls that previously "succeeded" by silently dropping arguments now error. Well-formed calls are unaffected.

Out of scope (parked for a security owner)

Policy-layer audit findings: audience allowlist non-authority (ToolAudienceProfileResolver), null path-token fail-closed in shell trust zones, safe-verb auto-grant audit logging. Also deferred: a typed out-of-band channel for the args-parse signal (currently an in-band sentinel key, blast radius bounded), and agent-loop / no-progress detection (separate workstream, same originating incident).

Validation

  • OpenSpec change loud-tool-arg-validation (proposal/design/specs/tasks), synced to main specs and archived.
  • Full solution suite 5,004 tests, 0 failures; dotnet slopwatch analyze clean; copyright headers verified.
  • Behavioral eval suite: Tool category 7/7 incl. a new tool_timeout_arg_recovery case (validated against Qwen3.6-35B on self-hosted vLLM — the same model class as the originating incident).
  • A /code-review high pass surfaced 10 findings; all are fixed in this branch (final commit), with regression tests.

Traceability

PRD-001 (MVP tool surface), PRD-002 (gateway security envelope); constitution "No silent fallbacks". Origin incident: memorizer memory e9a72b27-72d9-4e98-aad7-4d970ce52ecf.

Audit of the tool-call argument surface found 17 silent-discard/degrade
sites sharing three mechanisms: unknown keys dropped without signal,
present-but-invalid values coerced to defaults, and requested values
silently clamped or overridden. Origin: production session
D0AC6CKBK5K_1781115410_840529, where "TimeoutSeconds":"1200" was
silently dropped (only _timeout_seconds is recognized), the shell tool
fell back to its 90s default, and the agent's delegation call was
killed mid-flight, feeding a stuck loop.

The OpenSpec change (proposal, design, spec deltas, tasks) establishes
a no-silent-discard invariant at the tool-arg seam: unknown keys reject
with did-you-mean suggestions (fuzzy matching generates suggestion text
only, never acceptance), invalid values reject instead of coercing, and
every clamp/override emits a model-facing notice. Policy-layer findings
are parked as open questions for a security owner.
Implements the loud-tool-arg-validation OpenSpec change: LLM-supplied
tool arguments are never silently discarded, coerced, or overridden.

- Unknown-key validation at DispatchingToolExecutor (sync + streaming):
  unrecognized argument keys reject the call before execution with a
  "did you mean" suggestion and the valid argument names. Recognition
  mirrors actual consumption semantics — flexible (case/punctuation)
  for declared parameters, exact-only for meta keys. Fuzzy matching
  generates suggestion text only, never acceptance. MCP tools exempt
  (server-side schema validation is authoritative).
- Strict value binding: new GetIntStrict/GetDoubleStrict/GetBoolStrict
  helpers distinguish absent from present-but-invalid; the tool source
  generator emits them so invalid values reject instead of coercing to
  0/0.0/false. Non-integral numerics no longer silently truncate, and
  non-integral/overflow JSON numbers no longer throw uncaught.
- Meta-value validation: malformed _timeout_seconds/_background values
  reject pre-dispatch (pipeline-side; persisted ToolCallMeta unchanged).
- Override notices: timeout ceiling clamps and below-floor requests now
  surface in the tool result (appended post-bounding so they cannot be
  spilled away), steering to _background:true for longer work.
- Provider boundary: malformed tool-call arguments JSON travels as a
  sentinel and is rejected per call id instead of dispatching the tool
  with null arguments; deterministic on persistence re-drive.
- web_fetch: Format validated against {raw, text} (no silent raw
  fallback); 5 MB response-cap truncation is surfaced in the summary.
- list_webhooks: the schema-advertised Filter parameter is honored
  (active/all) instead of being a silent no-op.
- netclaw-operations skill v2.12.0: timeout-clamp and background-job
  delegation guidance, strict argument naming rules.
- New eval case tool_timeout_arg_recovery (passed 3/3 against
  Qwen3.6-35B on self-hosted vLLM); full suite 4,998 tests green.

Origin: production session D0AC6CKBK5K_1781115410_840529, where
"TimeoutSeconds":"1200" was silently dropped and the shell timeout
fell back to a 90s default, killing a delegation call mid-flight.
The test asserted a valid ShellTool call passed validation but never
touched an McpToolAdapter, leaving the "MCP tool exempt from native key
validation" spec scenario without real coverage. It now registers an
McpToolAdapter, dispatches a call with an unknown argument key, and
asserts the native "Unrecognized argument" rejection does not fire —
the unknown-key gate runs before authorization, so any other outcome
proves the exemption.
Sync delta specs to main specs:
- New capability spec tool-arg-validation (unknown-key rejection with
  suggestion-only near-miss matching, present-but-invalid value
  rejection, malformed args-JSON handling, override notices)
- tool-call-metadata: per-call timeout hint now surfaces clamp/floor
  overrides in the tool result; new malformed-meta-value rejection
  requirement
- netclaw-tools: web_fetch format validation + response-cap truncation
  notice; list_webhooks filter honored

Archive change to openspec/changes/archive/2026-06-11-loud-tool-arg-validation
(26/26 tasks, verification clean, eval suite 7/7).
Correctness:
- Accept integral JSON numbers written with a decimal point (300.0, 12.0)
  as integers; JsonElement.TryGetInt32 rejects that text, so coercion now
  falls back to an integral-double check (12.5 still rejected). Models
  commonly emit whole numbers as N.0, which previously hit a spurious
  "not a valid integer" rejection loop.
- Parse string-typed numeric arguments with InvariantCulture. The daemon
  runs with InvariantGlobalization=false, so a comma-decimal host locale
  previously misparsed or falsely rejected values like "1.5".
- Consolidate all pre-dispatch validation (provider args-parse sentinel,
  present-but-invalid meta values, unrecognized keys) into
  IToolExecutor.ValidateToolCall. The sub-agent loop calls the executor
  directly and previously skipped meta-value validation and the sentinel
  check — the silent-fallback gap this feature exists to close.
- Audit unknown-argument rejections as Allowed=false: the pipeline now
  preflights via ValidateToolCall instead of running the executor and
  mislogging the rejected call as executed.
- Accept the text<->Message parameter alias bidirectionally so a
  send_channel_message call using "Message" (which binding consumes) is
  no longer rejected. Alias defined once in ToolArgumentHelper.
- Clamp background-job timeouts to MaxToolTimeoutSeconds so _background
  cannot bypass the ceiling (was an unbounded kill timer).
- Bound the args-parse sentinel value (200 chars) so a forged/oversized
  value cannot flood the tool result.

Cleanup:
- One shared TryCoerceInt/Double/Bool + RenderValue across the strict
  binders, ToolCallMeta.ExtractFrom, and ValidateMetaValues (was three
  hand-mirrored ladders that had to stay in lockstep by hand).
- Delete the now-unused GetNullableInt/Double/Bool helpers.
- web_fetch streams into a pooled growing buffer: no eager 5 MB
  allocation per fetch, no 5 MB slice-copy on truncation.

Full suite 5,004 tests green; slopwatch clean; headers verified.
…ad-write-build-loop

# Conflicts:
#	feeds/skills/.system/files/netclaw-operations/SKILL.md

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found an immediate design issue that needs to be cleaned up - still working through the rest

/// to <c>channelid</c>. Shared with <see cref="ToolArgumentValidator"/> so
/// key recognition mirrors the exact matching that binding performs.
/// </summary>
public static string NormalizeKey(string key)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strip out special characters and whitespace - do this for both the tool name and the input

Comment thread src/Netclaw.Actors.Tests/Sessions/Pipelines/MetaValidationAndNoticeTests.cs Outdated
Per review: a per-call timeout is the agent's judgement, not something to
override. Clamping a requested _timeout_seconds down to a ceiling (and
flooring it up to a default) was friction with no clear benefit — the floor
especially (a shorter timeout is strictly safer), and the silent version was
the original fallback-rule violation this PR set out to fix.

- _timeout_seconds is now honored exactly: no ceiling, no floor, no notice.
  Absent → the inherited SessionConfig.ToolExecutionTimeout default applies.
- Background-job path honors the requested timeout too (was clamped).
- Collapse the conflicting timeout sources: remove ToolConfig.MaxToolTimeoutSeconds
  (the ceiling) and ToolConfig.ShellTimeoutSeconds (a 60s default shadowed in
  production by the 90s pipeline default). Single source of truth is
  SessionConfig.ToolExecutionTimeout; ShellTool keeps a const fallback only for
  the no-pipeline-context path. Both properties removed from
  netclaw-config.v1.schema.json (doctor --fix strips them from existing configs
  via additionalProperties:false).
- Delete the now-dead ToolExecutionContext.Notices mechanism (the clamp notice
  was its only producer) and ComputeEffectiveTimeout.
- Update tool-call-metadata spec, background-jobs runbook, configuration doc,
  and netclaw-operations skill (v2.12.1) to honor-the-hint semantics.

Tests updated: timeout hints honored exactly (high + low), background honors
requested timeout, ShellTool timeout driven via context. Core suites green
(Actors 2386, Configuration 416, Daemon 734, Cli 802); slopwatch + headers
clean; specs validate.
NuGetAudit flags MessagePack 2.5.192 (GHSA-hv8m-jj95-wg3x — LZ4 decompression
DoS), pulled in transitively only by the Aspire demo samples:
  Aspire.Hosting.AppHost → StreamJsonRpc 2.22.23 → MessagePack 2.5.192
Netclaw uses MessagePack in no production code, and the affected LZ4 path is
not exercised by Aspire's local tooling RPC — but the advisory DB update turned
it into a warning-as-error that fails pr_validation's solution build (dev is
affected identically). Pin to the patched v2 (2.5.301) via central package
management + a direct reference in the two sample projects to promote the
transitive. Same minor branch, no API risk for StreamJsonRpc.
@Aaronontheweb Aaronontheweb marked this pull request as ready for review June 12, 2026 18:31
@Aaronontheweb Aaronontheweb added tools Issues related to agent tools: file_read, web_search, shell_execute, image processing, etc. config Configuration issues, netclaw doctor, schema validation. reliability Retries, resilience, graceful degradation labels Jun 12, 2026

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - we'll have to see how it performs during beta testing. Passed evals with flying colors.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

}

[Fact]
public async Task Integral_decimal_json_timeout_accepted_and_executes()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - tolerant reader stuff

Assert.Equal(0, executor.Invocations);
Assert.Contains("'_background'", content);
Assert.Contains("boolean", content);
Assert.Contains("NOT executed", content);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

}

[Fact]
public async Task Non_integral_json_number_surfaces_parse_error_not_truncation()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

}

[Fact]
public async Task Valid_string_number_still_binds()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

[Theory]
[InlineData("yes")]
[InlineData("1")]
public void BoolStrict_colloquial_string_throws(string value)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We require true or false only.

if (arguments.TryGetValue("_timeout_seconds", out var tVal) && tVal is not null)
{
timeoutSeconds = tVal switch
// Shares ToolArgumentHelper.TryCoerceInt with ValidateMetaValues so

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

{
background = bVal switch
// Shares ToolArgumentHelper.TryCoerceBool with ValidateMetaValues.
if (ToolArgumentHelper.TryCoerceBool(bVal, out var parsedBackground))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb Aaronontheweb merged commit b71917e into netclaw-dev:dev Jun 12, 2026
15 checks passed
@Aaronontheweb Aaronontheweb deleted the investigate/agent-read-write-build-loop branch June 12, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config Configuration issues, netclaw doctor, schema validation. reliability Retries, resilience, graceful degradation tools Issues related to agent tools: file_read, web_search, shell_execute, image processing, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant