developmentseed / developmentseed/mcp-toolsets-runtime

An optional declared parameter that state cannot fill is dropped silently

Open
#83 2 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
0
Forks
0
Avg merge
1d 18h
Merged PRs (30d)
39

Description

An optional declared parameter that session state cannot fill is dropped silently. The tool runs without it and nothing — not the model, not the transcript, not the host — records that a value was expected and missing.

## Where this came from

ECMWF's `cds` toolset declares a bounding box:

```python
async def submit_request(
dataset: str,
request: dict[str, Any],
area: Annotated[list[float] | None, Kind(BBOX, model_generatable=False)] = None,
) -> ...
```

`get_aoi` publishes `gazet/bbox` as a `geo.BoundingBox`; the runtime injects it. Working as intended, a geometry reaches the request without ever entering the model's context.

A model that batches `get_aoi` and `submit_request` into **one** assistant message breaks it. LangGraph runs both in a single super-step against state as it stood at the start, so `submit_request` cannot see the publication happening beside it. `area` is absent, and the request submits **over the whole dataset instead of over Copenhagen**.

No error. CDS accepts a dataset-wide request, queues it, returns a job id. The geometry never appears in the conversation by design, so its absence does not either. The user gets data — the wrong data, at a much larger size, for somewhere they did not ask about.

## Why

`injection.py`:

```python
found = resolve(declaration, injected_state, schemas[parameter])
if found is not None:
arguments[parameter] = entry.get("value") # inject
elif declaration.get("required", True):
raise StateRefusal(...) # refuse
# implicit third branch: proceed without it, silently
```

And `required` is not something anyone chose. From `declarations.py`:

```python
# required is read from the tool's own input schema rather than declared
_declaration(parameter, marker, required=parameter not in optional)
```

`area: ... = None` has a default, so it is not in the schema's `required` list, so `required=False`. **The safety behaviour is coupled to an unrelated Python idiom** — the `= None` you have to write anyway, because the model is not supplying this parameter.

`required=True` is not the fix. Optional is correct here: dataset-wide requests are legitimate, and the publishing toolset may not be connected at all.

## Proposed fix: say so, do not refuse

`bind_injected` already computes, per parameter, which connected tools publish that kind:

```python
producers = {item["parameter"]: ... published.get(wants(item), []) for item in declarations}
```

That is exactly the missing distinction, and it is already in scope:

- **falsy** — nothing publishes this kind. Absence is expected. Stay silent.
- **truthy** — something publishes it and state is empty anyway. Suspicious.

So make the third branch explicit:

```python
elif publishers := producers[parameter]:
omitted[parameter] = Omission(kind=wants(declaration), publishers=publishers)
```

and surface it through machinery that already exists:

- **to the model**, as a third breadcrumb beside `[state used: …]` and `[state updated: …]`:
`[state missing: area — geo.BoundingBox, published by get_aoi, not yet in session state]`
- **to a host**, as a `state.omitted` activity beside `state.consumed` / `state.published`.

The breadcrumb is the point: it lands in the transcript in the same turn, so the model can resubmit with the AOI or tell the user the request went dataset-wide. Same recovery shape as #74's refusal-as-`ToolMessage`.

A refusal instead would be worse — on turn one a legitimate dataset-wide request would be blocked, and the model has no escape hatch, since `model_generatable=False` prunes the parameter from its schema entirely. It cannot say "I meant the whole domain". Annotation is silent in exactly the case that is legitimate.

## Notes

- `Omission` should be a sibling of `Receipt`, not a `Receipt` — there is no `key`, nothing was read, and `receipts_of` filters on `key` being truthy.
- `_residue` rewrites the artifact after capture and carries receipts through explicitly. Omissions need the same, or they are lost between injection and the message. Worth a test that asserts the omission survives *capture*, not just injection.
- Adding `state.omitted` changes the documented activity vocabulary, so `CONSUMING.md`, `SESSION-STATE.md` and the example README need a line.
- **This makes the mistake visible, it does not prevent it.** The batched call still submits without the area. Preventing it means not running a consumer and its publisher in the same super-step, which is a graph-level change and a separate issue.

Contributor guide

Open the contributing guide

Research direction

Start in injection.py and declarations.py, then trace bind_injected, the existing Receipt and activity machinery, and _residue. Add omission handling only when a connected publisher exists, surface it to the model and host, and preserve it through capture. Verify the captured omission and update CONSUMING.md, SESSION-STATE.md, and the example README with the new state.omitted activity.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.