[ViperJuice]

Addresses the five catalog-indexing findings that were split out of #173 so
that issue could be resolved on its own subject. See #175.

Implements .consiliency/plans/detailed-catalog-indexing-hardening-20260827-1745.md
(revision 2). Landed in the plan's order — item 1's test first against unchanged
code, then 3 and 5, then 2, and item 4 last and alone, since it is the only
behaviour change and a bisect should reach it cleanly.

Item 1 — the truncation boundary, and its mutation proof

This is the item with teeth. Mutating the guard len(entries) >= limit to
> limit survived the entire existing test file and was the sole survivor
of nine mutants. That off-by-one lets a downstream put limit + 1 tools in the
catalog — one more than the bound, every time.

The test was written and committed against unchanged code (fde84cc) before
the mutation was applied, so it is a proof rather than a coincidence. Recorded
exit codes:

step	result	exit
unchanged code	4 passed	0
>= limit → > limit	2 failed, 2 passed	1
mutation reverted (git diff empty)	4 passed	0

(The unchanged-code run printed its pass count but its exit status was not
captured numerically; the 0 is documented by the restored run, which is on a
byte-identical tree — git diff was empty at that point.)

The mutant fails on the limit + 1 case with assert 6 == 5. Only that case
distinguishes >= from >, which is why the test asserts exact counts at
limit - 1, limit and limit + 1 rather than merely "fewer than offered".

Item 3 — a zero limit is not the server's fault, and adds no schema bound

max_tools_per_server: 0 empties the parse result before a single entry is
examined, so reconciliation announced "Every tools entry in the listing was
unparseable" — an accusation aimed at the downstream for a decision the
gateway's own policy file made. Both that message and the parser's truncation
warning now name the limit.

This deliberately does not add Field(ge=1) to LimitsPolicy, and that is
not an oversight. Revision 1 of the plan proposed exactly that, and it is a
fail-open security regression: PolicyManager._load_policy(..., fatal=False) —
the auto-discovery path — swallows any validation exception and leaves the
default allow-all GatewayPolicy in place (policy/policy.py:59-77). A bound
here would make any policy file containing max_tools_per_server: 0
schema-invalid and silently discard the whole file — allowlist, denylist,
limits and redaction all reverting to permissive. Trading a cosmetic log fix for
that is a spectacularly bad deal.

The underlying fail-open is filed separately as #202 and is not this
change's to fix; folding it in would hide it. A test pins that
LimitsPolicy(max_tools_per_server=0) still validates, and a docstring on
the field records why, so a future tightening cannot land here unnoticed.

Item 5 — the count is of what landed

_index_resources documented that "the count returned is what was actually
indexed, not what was offered, so a caller reporting it is not overstating the
catalog" while all three _index_* returned len(entries). Two entries
sharing an identity are two list items and one catalog key, so the promise was
false precisely when identities collide — and a documented guarantee
contradicted by the code is worse than an undocumented gap, because a reader
trusts it. The count is now of distinct identities, and each collision is logged
at DEBUG naming the id: the honest count makes last-write-wins visible, the
log makes it diagnosable.

One course correction worth reading (b5d0609). The first cut moved the
write and the count into a module-level _apply_entries(catalog, ...) helper.
tests/runtime/test_publisher_coverage.py caught it: that is an AST honesty
guard which attributes every self._tools/self._resources/self._prompts
write to its enclosing method and fails if one appears outside the five
publishing mutators. A helper taking the dict as a parameter is invisible to
that walk — so the refactor did not trip the guard, it silently emptied it,
leaving the three _index_* methods looking as though they no longer write to a
catalog at all. The write went back where the guard can see it; the helper
(_distinct_indexed) now only counts and logs, and its docstring records why it
must not take the dict, so the next reader does not re-make the simplification.

Item 2 — adopt_process clears first

_connect_stdio, _reconcile_once and _cleanup_client all remove a server's
entries before indexing; adopt_process was the odd one out, so adopting a
server previously indexed under the same name left the earlier listing's tools
in the catalog beside the new ones — entries the adopted process does not serve,
still routable. Made uniform rather than documented as an exception.

Asserted on catalog contents, not on whether the removal method was called —
a call-count assertion passes just as happily if the removal runs after the
index. Verified with teeth: dropping the new line reds two of the four tests
(exit 1), restoring it greens them (exit 0).

Item 4 — behaviour change: a tool with no inputSchema is skipped

This is the only behaviour change in the set and is called out in the
CHANGELOG under ### Changed, not buried in ### Fixed.

tool.get("inputSchema", {}) manufactured a schema the server never sent, and
{} does not mean "we do not know" — it means "any arguments at all are valid",
published under that server's name to every caller and every model reading the
catalog. MCP requires inputSchema on a tool, so a tool without one is a tool
we could not read, and #172 already settled what happens to those.

The check is a new _required_object, a sibling of _required_identity
rather than a flag on it: that helper requires isinstance(value, str) and value, so calling it for an object-valued field would reject every valid tool
in the catalog. It raises inside the parser's existing per-entry try, so a
schemaless tool costs only itself, and a listing of nothing but such entries
lands in _reconcile_once's offered-but-none-parseable rule — prior tools kept,
nothing published as removed. An explicit inputSchema: {} is still accepted;
absence, null and any other non-object are not.

Fixture fallout was confined to the six lines in
tests/mcp2x/test_catalog_publishers.py the plan predicted (130, 140, 193, 218,
278, 310). The full suite surfaced no other schemaless tool fixture.

One deviation from the plan, stated rather than hidden

The plan's Documentation impact says "one ### Fixed entry naming all five".
The CHANGELOG instead splits them across ### Changed (item 4), ### Fixed
(items 2, 3, 5) and ### Added (item 1). That is better Keep-a-Changelog
semantics and serves the plan's own instruction that item 4 be "called out as
such, not buried" — but it is a deviation, so it is noted here rather than left
for a reviewer to trip on.

Verification

command	result	exit
uv run pytest -q	3217 passed, 3 skipped, 25 deselected in 352.93s	0
uv run ruff check src/ tests/	All checks passed	0
uv run ruff format --check src/ tests/	118 files already formatted	0
uv run mypy src/	Success: no issues found in 43 source files	0

The 28 tests added across the five items pass as a group
(-k "TestToolLimitIsEnforced or TestDuplicateIdentities or TestZeroLimitLogsAccurately or TestAdoptProcess or TestMissingInputSchema").

CI on this PR: 12/12 checks pass — test (3.10) 5m27s, test (3.11) 5m15s,
test (3.12) 4m35s, plus lint, typecheck, build, changelog, install-smoke,
min-version-smoke, workflows, release-diff-ack, notify-worker.

No manifest or npm-resolver path is touched — git diff main --name-only over
manifest|resolver|npm returns nothing — so the 98-server manifest resolution
is unchanged. (Revision 1 listed a 98-server manifest check as verification;
revision 2 removed it as unrelated to catalog indexing and not coverage for
dropped tools, so it is not re-invented here.)

🤖 Generated with Claude Code

https://claude.ai/code/session_018W2e4mGLxCfQeKqQ1P6CPB