feat(validate-agents): assert that a claimed room actually exists - #2626
Conversation
The registry already cross-checks rooms -> registry ("does this room's owner
exist"). The mirror was missing, and a fabricated name shipped because of it:
PR #2612 bound pmoves_minimax_mcp to "minimax-mcp.room.model", which appears in
no catalog and no manifest. The binding resolved to nothing, and the agent was
undiscoverable through the room plane while looking correctly wired.
Adds the symmetric check: every `rooms:` entry must name a room present in
pmoves/config/rooms/catalog.json or on disk as a manifest. Advisory, matching
the owner check beside it.
Scoped deliberately to what is unambiguous. `rooms:` has no schema, and whether
a room manifest should reciprocate by declaring the agent's action_namespace is
unsettled -- 4 of the 7 rooms-bound entries (cipher, nats, tailscale, minimax)
do not reciprocate, and `action_namespace` is OPTIONAL in
room.manifest.v1.schema.json. So reciprocity is an absent contract, not a
violated one, and is left to the owners. "The room must exist" needs no such
decision.
VERIFIED IT CAN SAY NO, and the first version could not:
clean main -> no rooms-entry warnings
fabricated room reinjected -> WARN mcp_servers[pmoves_minimax_mcp]: rooms
entry 'minimax-mcp.room.model' is in neither
catalog.json nor a room manifest
restored -> no rooms-entry warnings
The first draft scanned `agents:` only and caught NOTHING on that middle case --
every `rooms:` occurrence today is under `mcp_servers:`. It now scans both, so
the check cannot go quiet if the field moves or spreads. Shipping that draft
would have added a gate that reports fine while measuring nothing, which is the
defect class this session has spent its time removing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcd49253ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for manifest in ROOMS_DIR.glob("*.json"): | ||
| if manifest.name != "catalog.json": | ||
| known_rooms.add(manifest.name[: -len(".json")]) |
There was a problem hiding this comment.
When a manifest filename differs from its embedded room_id, this adds a nonexistent alias to known_rooms. For example, demo.room.json declares demo.room.rehearsal, so a broken registry claim for demo.room passes silently; sidecars such as demo.room.extras.json similarly make demo.room.extras appear valid even though the canonical manifest validator excludes *.extras.json. Because P7 resolves rooms by catalog/manifest room_id, these claims remain undiscoverable—the exact defect this check is intended to catch. Parse the manifest's room_id and exclude sidecars rather than using the filename stem.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
1 participant
Closes the third of the three follow-ups from the merge sweep.
The gap
validate_agent_registry.pyalready cross-checks rooms → registry: "does this room's owner resolve to a registry agent?" The mirror was missing, and a fabricated name shipped because of it.PR #2612 bound
pmoves_minimax_mcpto"minimax-mcp.room.model"— a room in no catalog and no manifest. The binding resolved to nothing: the agent was undiscoverable through the room plane while looking correctly wired. Nothing caught it but a reviewer reading the diff.This adds the symmetric check — every
rooms:entry must name a room present incatalog.jsonor on disk as a manifest. Advisory, matching the owner check beside it.Scoped to what is unambiguous
Deliberately not checking reciprocity — whether a room manifest should declare the agent's
action_namespaceback. I measured before deciding:cipher,nats,tailscale,minimax)action_namespaceis optional inroom.manifest.v1.schema.jsonrooms:at allSo reciprocity is an absent contract, not a violated one. Settling it means deciding what
rooms:means — a reference the room resolves, or a two-sided contract — and that is the owners' call. "The room must exist" needs no such decision.Verified it can say no — and the first version could not
My first draft scanned
agents:only and caught nothing on that middle case — everyrooms:occurrence today lives undermcp_servers:. It now scans both, so the check cannot go quiet if the field moves or spreads.Worth stating plainly: shipping that draft would have added a gate that reports fine while measuring nothing — the exact defect class this session has spent its time removing. The injection test is the only reason I know the difference.