diff --git a/README.md b/README.md index c31bd62..d2ba2f2 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,30 @@ writes are split by naming it twice — the same `url` and `connection`, two lists of tool names, two effects. Where a server hands out one grant for both, that is the only place an agent can say it means to read. +With more than one engine, name them once in `gete.yaml` and let each agent +refer to one by name: + +```yaml +# gete.yaml +gemini_enterprise: + engines: + sales: my-sales-app_1234567890 + support: my-support-app_1234567891 +``` + +```yaml +# agents/partner-review/agent.yaml +registration: + gemini_enterprise: + engine: sales +``` + +Once the engines are named, every `engine` is one of the names: `validate` +refuses a name no engine carries, and refuses an id even when it is right, +since accepting both would let an id copied from the agent next door land on +the wrong engine unnoticed. Each id is written once, so a rebuilt engine is +one line, and `gete graph` and `gete register` say the name. + ### Tools from an OpenAPI description A service that publishes an OpenAPI description but runs no MCP server can be diff --git a/docs/quickstart.md b/docs/quickstart.md index 076b9e6..2710218 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -212,7 +212,9 @@ registration: engine: my-app_1234567890 ``` -Then: +With several engines, name them once in `gete.yaml` under +`gemini_enterprise.engines` and write the name here instead; the README +shows the shape. Then: ```sh gete register diff --git a/examples/minimal/agents/minimal/agent.yaml b/examples/minimal/agents/minimal/agent.yaml index 55c175e..221ee75 100644 --- a/examples/minimal/agents/minimal/agent.yaml +++ b/examples/minimal/agents/minimal/agent.yaml @@ -8,7 +8,9 @@ starter_prompts: - Summarize the text I paste in three bullet points. # Without a registration block the agent is deployed to Agent Engine but not -# listed in Gemini Enterprise. Add the engine id from the console URL to list it: +# listed in Gemini Enterprise. To list it, add the engine id from the console +# URL, or its name once gete.yaml names the engines under +# gemini_enterprise.engines: # registration: # gemini_enterprise: # engine: my-app_1234567890 diff --git a/src/gete/declaration.py b/src/gete/declaration.py index 80b27fe..2903292 100644 --- a/src/gete/declaration.py +++ b/src/gete/declaration.py @@ -128,6 +128,17 @@ def instruction_text(self) -> str: path = self.instruction_path return path.read_text(encoding="utf-8") if path else self.instruction + @property + def engine(self) -> str | None: + """What registration.gemini_enterprise.engine says, or None when unlisted. + + A name from gemini_enterprise.engines when gete.yaml declares them, + the engine's id otherwise; Project.engine_id tells which. + """ + registration: Mapping[str, Any] = self.data.get("registration", {}) + engine = registration.get("gemini_enterprise", {}).get("engine") + return str(engine) if engine else None + @property def source(self) -> Path | None: value = self.data.get("source") @@ -173,6 +184,32 @@ def connection_overrides(self) -> Mapping[str, Mapping[str, Any]]: overrides: Mapping[str, Mapping[str, Any]] = self.data.get("connections", {}) return overrides + @property + def engines(self) -> Mapping[str, str] | None: + """Engine ids by name, or None while agents write the ids themselves.""" + gemini_enterprise: Mapping[str, Any] = self.data.get("gemini_enterprise", {}) + engines: Mapping[str, str] | None = gemini_enterprise.get("engines") + return engines + + def engine_id(self, declared: str) -> str: + """The id behind what an agent wrote as its engine. + + With engines named, the entry is one of the names and nothing else. + An id is refused even when it is right: accepting both would let an + id copied from the agent next door land on the wrong engine + unnoticed, which naming the engines exists to stop. + """ + if self.engines is None: + return declared + if declared not in self.engines: + names = ", ".join(self.engines) + raise DeclarationError( + f"registration.gemini_enterprise.engine: {declared!r} is not " + f"among gemini_enterprise.engines in {PROJECT_FILE} ({names}); " + "with the engines named, an agent refers to one by name" + ) + return self.engines[declared] + def display(self, path: Path) -> str: """Path as shown in messages: relative to the project root when below it.""" try: diff --git a/src/gete/graph.py b/src/gete/graph.py index 62fe366..5593117 100644 --- a/src/gete/graph.py +++ b/src/gete/graph.py @@ -1,12 +1,10 @@ """gete graph: Mermaid drawn from the declarations; no second diagram to go stale.""" import re -from collections.abc import Mapping -from typing import Any from urllib.parse import urlsplit from gete.connection import Registry -from gete.declaration import Agent, Project +from gete.declaration import Project from gete.errors import GeteError # Marks what acts with a credential the agent holds rather than the caller's. @@ -38,7 +36,7 @@ def mermaid(project: Project, names: list[str] | None = None) -> str: if names and agent.name not in names: continue node = _ident(agent.name) - engine = _engine(agent) + engine = agent.engine if engine and engine not in engines: engines.add(engine) lines.append( @@ -110,11 +108,5 @@ def _via(registry: Registry, connection_id: str) -> str: return connection_id + (BOT if connection.app is not None else "") -def _engine(agent: Agent) -> str | None: - registration: Mapping[str, Any] = agent.data.get("registration", {}) - engine = registration.get("gemini_enterprise", {}).get("engine") - return str(engine) if engine else None - - def _ident(text: str) -> str: return re.sub(r"[^A-Za-z0-9_]", "_", text) diff --git a/src/gete/register.py b/src/gete/register.py index ae5d715..f9aa93c 100644 --- a/src/gete/register.py +++ b/src/gete/register.py @@ -250,8 +250,8 @@ def run(self, names: list[str] | None = None, reset: Sequence[str] = ()) -> Summ for agent in self._project.agents: if names and agent.name not in names: continue - engine = _engine_id(agent) - if engine is None: + written = agent.engine + if written is None: summary.say( f"{agent.name}: no registration.gemini_enterprise.engine; skipped" ) @@ -261,6 +261,12 @@ def run(self, names: list[str] | None = None, reset: Sequence[str] = ()) -> Summ identifier for identifier in reset if owners[identifier] == agent.name ] try: + # Resolved before anything is read: a name no engine carries + # is the copy-paste the names exist to refuse, and register + # can run without validate. + engine = self._project.engine_id(written) + behind = "" if engine == written else f" ({engine})" + summary.say(f"{agent.name}: Gemini Enterprise engine {written}{behind}") self._register(agent, engine, summary, held) except GeteError as error: summary.say(f"{agent.name}: cannot register: {error}") @@ -311,9 +317,7 @@ def _reset_owners( f"the agent is not among {', '.join(names)}" ) unregistered = sorted( - identifier - for identifier in reset - if _engine_id(declared[identifier]) is None + identifier for identifier in reset if declared[identifier].engine is None ) if unregistered: raise DeclarationError( @@ -713,12 +717,6 @@ def _fill(template: str, **values: str) -> str: ) from None -def _engine_id(agent: Agent) -> str | None: - registration: Mapping[str, Any] = agent.data.get("registration", {}) - engine = registration.get("gemini_enterprise", {}).get("engine") - return str(engine) if engine else None - - def register_project( project: Project, gcp: GcpApi, diff --git a/src/gete/schema/agent.json b/src/gete/schema/agent.json index 0e105c3..c396cfd 100644 --- a/src/gete/schema/agent.json +++ b/src/gete/schema/agent.json @@ -146,7 +146,7 @@ ], "properties": { "engine": { - "description": "Id of the Gemini Enterprise engine. Spliced into the Discovery Engine URL path, so a slash would address another resource.", + "description": "The Gemini Enterprise engine: its id from the console URL, or its name once gete.yaml names the engines under gemini_enterprise.engines. The id is spliced into the Discovery Engine URL path, so a slash would address another resource.", "type": "string", "pattern": "^[a-zA-Z0-9][a-zA-Z0-9_-]*$" } diff --git a/src/gete/schema/gete.json b/src/gete/schema/gete.json index 5744280..2fe444b 100644 --- a/src/gete/schema/gete.json +++ b/src/gete/schema/gete.json @@ -39,6 +39,19 @@ "description": "Where Gemini Enterprise holds the engine, usually global. Becomes a path segment of every Discovery Engine URL.", "type": "string", "pattern": "^[a-z]+(-[a-z0-9]+)*$" + }, + "engines": { + "description": "The engines by name, each the id from the console URL. Once declared, every agent's registration.gemini_enterprise.engine is one of these names and nothing else: an id written there is refused, so one copied from the agent next door cannot land on the wrong engine unnoticed. Each id is written once, so a rebuilt engine is one line here.", + "type": "object", + "minProperties": 1, + "propertyNames": { + "pattern": "^[a-z][a-z0-9-]*$" + }, + "additionalProperties": { + "description": "Id of the engine. Spliced into the Discovery Engine URL path, so a slash would address another resource.", + "type": "string", + "pattern": "^[a-zA-Z0-9][a-zA-Z0-9_-]*$" + } } } }, diff --git a/src/gete/secrets.py b/src/gete/secrets.py index e4672b4..fa52e0a 100644 --- a/src/gete/secrets.py +++ b/src/gete/secrets.py @@ -28,7 +28,7 @@ def secrets_needed(project: Project) -> dict[str, list[str]]: # Delivered like secret_env too, registered or not: the deployment # issues its tokens with the key. names.extend(app_key_secrets(project, agent).values()) - if agent.data.get("registration"): + if agent.engine is not None: for connection_id in agent.connections: connection = registry.get(connection_id) if connection.oauth is None: diff --git a/src/gete/templates/agent.yaml b/src/gete/templates/agent.yaml index 6cc9645..2b3be8f 100644 --- a/src/gete/templates/agent.yaml +++ b/src/gete/templates/agent.yaml @@ -15,7 +15,8 @@ instruction: ./instruction.md # effect: read # does_not: Does not create or approve anything. -# Where the agent is listed. The id is in the Gemini Enterprise console URL. +# Where the agent is listed: the id from the Gemini Enterprise console URL, +# or its name once gete.yaml names the engines under gemini_enterprise.engines. # registration: # gemini_enterprise: # engine: my-app_1234567890 diff --git a/src/gete/templates/gete.yaml b/src/gete/templates/gete.yaml index 1c1f3a4..4d09995 100644 --- a/src/gete/templates/gete.yaml +++ b/src/gete/templates/gete.yaml @@ -14,3 +14,9 @@ policies: # connections: # github: # base_url: https://api.github.example.com + +# Gemini Enterprise engines by name, each the id from the console URL. Once +# named, an agent's registration.gemini_enterprise.engine is one of the names. +# gemini_enterprise: +# engines: +# main: my-app_1234567890 diff --git a/src/gete/validate.py b/src/gete/validate.py index e8f2ef2..50080ea 100644 --- a/src/gete/validate.py +++ b/src/gete/validate.py @@ -38,6 +38,7 @@ def validate_project(project: Project) -> list[Problem]: found: list[Problem] = list(project.problems) registry = _registry(project, found) found.extend(_policy_problems(project)) + found.extend(_engine_problems(project)) for agent in project.agents: found.extend(_agent_problems(project, agent, registry)) return found @@ -84,6 +85,22 @@ def _policy_problems(project: Project) -> list[Problem]: return found +def _engine_problems(project: Project) -> list[Problem]: + """Two names for one engine: the block exists to make that copy visible.""" + names_by_id: dict[str, list[str]] = {} + for name, engine in (project.engines or {}).items(): + names_by_id.setdefault(engine, []).append(name) + return [ + Problem( + project.display(project.path), + f"gemini_enterprise.engines: {', '.join(names)} name the same " + f"engine, {engine}", + ) + for engine, names in names_by_id.items() + if len(names) > 1 + ] + + def _agent_problems( project: Project, agent: Agent, registry: Registry ) -> list[Problem]: @@ -167,6 +184,11 @@ def _agent_problems( found.extend( f"connections: {message}" for message in elimination_problems(known, registry) ) + if agent.engine is not None: + try: + project.engine_id(agent.engine) + except DeclarationError as error: + found.append(str(error)) declared_shared: Mapping[str, Any] = project.data.get("shared_credentials", {}) for name in agent.shared_credentials: credential = SHARED_CREDENTIALS.get(name) diff --git a/tests/test_graph.py b/tests/test_graph.py index e23f86c..d6393cb 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -199,3 +199,25 @@ def test_a_user_authorized_connection_is_not_drawn_as_a_bot( ) -> None: project.write_agent("finance", {"connections": ["freee"]}) assert "bot" not in graph(project) + + +def test_the_engine_is_drawn_by_its_name_when_gete_yaml_names_it( + project: ProjectBuilder, +) -> None: + """The id is what naming the engines takes out of sight; the diagram + follows suit, and the node is a plain identifier again.""" + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "gemini_enterprise": {"engines": {"sales": "my-sales-app_1234567890"}}, + } + ) + project.write_agent( + "finance", {"registration": {"gemini_enterprise": {"engine": "sales"}}} + ) + text = graph(project) + assert 'GE_sales["Gemini Enterprise
sales"]' in text + assert "GE_sales --> finance" in text + assert "my-sales-app_1234567890" not in text diff --git a/tests/test_register.py b/tests/test_register.py index 87145a5..7dd5638 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -1180,3 +1180,71 @@ def test_an_app_connection_has_no_authorization_body() -> None: claiming users approve a connection nobody authorizes.""" with pytest.raises(DeclarationError, match="issues its own tokens"): authorization_body(GE, "finance", CATALOG.get("github-app"), "c", "s") + + +# --- engines named in gete.yaml + +NAMED_ENGINES: dict[str, Any] = { + "version": 1, + "project": "example-project", + "location": "us-central1", + "gemini_enterprise": {"project_number": NUMBER, "engines": {"sales": "app_1"}}, +} + + +def test_an_engine_named_in_gete_yaml_is_resolved_to_its_id( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + project.write_project(NAMED_ENGINES) + project.write_agent( + "finance", + {**FINANCE, "registration": {"gemini_enterprise": {"engine": "sales"}}}, + ) + summary = register_project( + load_project(project.root / "gete.yaml"), gcp, tmp_path / "n.md" + ) + assert summary.failed == [] + assert summary.needs_human == ["finance"] + # The agent list is read under the id, and the notice sends the person there. + assert any(url == AGENTS_URL for _, url, _, _ in gcp.calls) + assert "engines/app_1/" in (tmp_path / "n.md").read_text() + + +def test_the_summary_names_the_engine_and_the_id_behind_it( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + project.write_project(NAMED_ENGINES) + project.write_agent( + "finance", + {**FINANCE, "registration": {"gemini_enterprise": {"engine": "sales"}}}, + ) + summary = register_project( + load_project(project.root / "gete.yaml"), gcp, tmp_path / "n.md" + ) + assert "finance: Gemini Enterprise engine sales (app_1)" in summary.messages + + +def test_the_summary_names_the_engine_by_id_when_that_is_what_the_agent_wrote( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + summary = register_project(project_with(project, FINANCE), gcp, tmp_path / "n.md") + assert "finance: Gemini Enterprise engine app_1" in summary.messages + + +def test_a_name_no_engine_carries_fails_that_agent_before_anything_is_read( + project: ProjectBuilder, gcp: FakeGcp, tmp_path: Path +) -> None: + """register can run without validate, and the id written here is exactly + the copy-paste naming the engines exists to refuse.""" + project.write_project(NAMED_ENGINES) + project.write_agent( + "finance", + {**FINANCE, "registration": {"gemini_enterprise": {"engine": "app_1"}}}, + ) + summary = register_project( + load_project(project.root / "gete.yaml"), gcp, tmp_path / "n.md" + ) + assert summary.failed == ["finance"] + assert any("'app_1'" in line and "sales" in line for line in summary.messages) + assert gcp.calls == [] + assert not (tmp_path / "n.md").exists() diff --git a/tests/test_schema.py b/tests/test_schema.py index 1e637f7..88eb54f 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -120,6 +120,49 @@ def test_gemini_enterprise_location_must_look_like_a_region(location: str) -> No ) +def test_gemini_enterprise_engines_are_named_once_in_gete_yaml() -> None: + validate_document( + "gete", + { + **GETE, + "gemini_enterprise": { + "engines": {"sales": "my-sales-app_1234567890", "support": "app_2"} + }, + }, + source="gete.yaml", + ) + + +@pytest.mark.parametrize("name", ["Sales", "my_sales", "-sales", "sales.eu", ""]) +def test_an_engine_name_is_shaped_like_a_connection_id(name: str) -> None: + """One shape for every short name a declaration refers to by.""" + with pytest.raises(DeclarationError, match="engines"): + validate_document( + "gete", + {**GETE, "gemini_enterprise": {"engines": {name: "app_1"}}}, + source="gete.yaml", + ) + + +@pytest.mark.parametrize("engine", ["../../../authorizations", "app 1", "app/1", ""]) +def test_a_named_engines_id_must_still_be_an_identifier(engine: str) -> None: + """The id goes into the Discovery Engine URL path wherever it is written.""" + with pytest.raises(DeclarationError, match="engines"): + validate_document( + "gete", + {**GETE, "gemini_enterprise": {"engines": {"sales": engine}}}, + source="gete.yaml", + ) + + +def test_an_empty_engines_block_is_refused() -> None: + """Declared but naming nothing, it would refuse every agent's engine.""" + with pytest.raises(DeclarationError, match="engines"): + validate_document( + "gete", {**GETE, "gemini_enterprise": {"engines": {}}}, source="gete.yaml" + ) + + def test_registration_engine_accepts_a_console_id() -> None: validate_document( "agent", diff --git a/tests/test_validate.py b/tests/test_validate.py index 7c636c1..09ebeb6 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -1035,3 +1035,79 @@ def test_an_openapi_block_reads_through_an_app_connection( connections=["github-app"], ) assert problems(project) == [] + + +# Engines named once in gete.yaml; agents refer to one by name. +NAMED_ENGINES: dict[str, Any] = { + "version": 1, + "project": "example-project", + "location": "us-central1", + "gemini_enterprise": { + "engines": {"sales": "my-sales-app_1234567890", "support": "app_2"} + }, +} + + +def registered_with(engine: str) -> dict[str, Any]: + return {"registration": {"gemini_enterprise": {"engine": engine}}} + + +def test_an_agent_registers_with_an_engine_by_its_name( + project: ProjectBuilder, +) -> None: + project.write_project(NAMED_ENGINES) + project.write_agent("mail-triage", registered_with("sales")) + assert problems(project) == [] + + +def test_a_name_no_engine_carries_is_refused_and_the_names_are_listed( + project: ProjectBuilder, +) -> None: + project.write_project(NAMED_ENGINES) + project.write_agent("mail-triage", registered_with("sale")) + found = problems(project) + assert len(found) == 1, found + assert "registration.gemini_enterprise.engine" in found[0] + assert "'sale'" in found[0] + assert "sales, support" in found[0] + + +def test_once_engines_are_named_an_id_is_refused_even_a_right_one( + project: ProjectBuilder, +) -> None: + """Accepting both would let an id copied from the agent next door land on + the wrong engine unnoticed, which naming the engines exists to stop.""" + project.write_project(NAMED_ENGINES) + project.write_agent("mail-triage", registered_with("my-sales-app_1234567890")) + found = problems(project) + assert any("my-sales-app_1234567890" in p and "by name" in p for p in found), found + + +def test_without_named_engines_an_agent_still_gives_the_id( + project: ProjectBuilder, +) -> None: + project.write_agent("mail-triage", registered_with("my-sales-app_1234567890")) + assert problems(project) == [] + + +def test_an_agent_without_a_registration_is_fine_whether_engines_are_named_or_not( + project: ProjectBuilder, +) -> None: + project.write_project(NAMED_ENGINES) + project.write_agent("mail-triage") + assert problems(project) == [] + + +def test_two_names_for_one_engine_are_reported(project: ProjectBuilder) -> None: + """The second name is the copy-paste the block exists to make visible.""" + project.write_project( + { + **NAMED_ENGINES, + "gemini_enterprise": {"engines": {"sales": "app_1", "support": "app_1"}}, + } + ) + project.write_agent("mail-triage", registered_with("sales")) + found = problems(project) + assert len(found) == 1, found + assert found[0].startswith("gete.yaml: gemini_enterprise.engines") + assert "sales" in found[0] and "support" in found[0] and "app_1" in found[0]