-
Notifications
You must be signed in to change notification settings - Fork 8
feat(registration): report the agent's commit and source repo at registration #508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,8 @@ | |
|
|
||
| from agentex.lib.utils.logging import make_logger | ||
| from agentex.lib.environment_variables import EnvironmentVariables | ||
| from agentex.lib.utils.build_provenance import normalize_remote | ||
| from agentex.lib.core.tracing.code_revision import is_git_object_name | ||
|
|
||
| logger = make_logger(__name__) | ||
|
|
||
|
|
@@ -20,6 +22,29 @@ def get_auth_principal(env_vars: EnvironmentVariables): | |
| except Exception: | ||
| return None | ||
|
|
||
|
|
||
| def build_registration_metadata(env_vars: EnvironmentVariables, agent_card=None) -> dict: | ||
| """Deployment id, source provenance, and agent card; keys appear only when known.""" | ||
| metadata: dict = {} | ||
| if env_vars.AGENTEX_DEPLOYMENT_ID: | ||
| metadata["deployment_id"] = env_vars.AGENTEX_DEPLOYMENT_ID | ||
| commit = (env_vars.AGENT_COMMIT_SHA or "").strip() | ||
| if commit: | ||
| if is_git_object_name(commit): | ||
| metadata["commit_sha"] = commit | ||
| else: | ||
| logger.warning( | ||
| "AGENT_COMMIT_SHA=%r is not a git commit SHA; commit_sha omitted from registration.", | ||
| commit, | ||
| ) | ||
| repo = normalize_remote(env_vars.AGENT_SOURCE_REPO) | ||
| if repo: | ||
| metadata["source_repo"] = repo | ||
|
Comment on lines
+40
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If How this was verified: The environment value reaches the registration payload through Prompt To Fix With AIThis is a comment left during a code review.
Path: src/agentex/lib/utils/registration.py
Line: 40-42
Comment:
**Repository Credentials Can Leak**
If `AGENT_SOURCE_REPO` contains a credential in its query string or fragment, such as `https://github.com/org/repo?access_token=SECRET`, `normalize_remote` preserves that suffix. The credential is then sent in `source_repo` and included in the successful-registration log. Strip query strings and fragments before adding the repository to the metadata.
**How this was verified:** The environment value reaches the registration payload through `normalize_remote`, whose string processing never removes `?` or `#` suffixes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid. Fixed at the layer that promises it: 🤖 — posted via Claude Code |
||
| if agent_card is not None: | ||
| metadata["agent_card"] = agent_card.model_dump() if hasattr(agent_card, "model_dump") else agent_card | ||
| return metadata | ||
|
|
||
|
|
||
| async def register_agent(env_vars: EnvironmentVariables, agent_card=None): | ||
| """Register this agent with the Agentex server""" | ||
| if not env_vars.AGENTEX_BASE_URL: | ||
|
|
@@ -33,13 +58,7 @@ async def register_agent(env_vars: EnvironmentVariables, agent_card=None): | |
| or f"Generic description for agent: {env_vars.AGENT_NAME}" | ||
| ) | ||
|
|
||
| # Registration metadata carries the deployment id and agent card. | ||
| registration_metadata: dict = {} | ||
| if env_vars.AGENTEX_DEPLOYMENT_ID: | ||
| registration_metadata["deployment_id"] = env_vars.AGENTEX_DEPLOYMENT_ID | ||
| if agent_card is not None: | ||
| card_data = agent_card.model_dump() if hasattr(agent_card, "model_dump") else agent_card | ||
| registration_metadata["agent_card"] = card_data | ||
| registration_metadata = build_registration_metadata(env_vars, agent_card) | ||
|
|
||
| # Prepare registration data | ||
| registration_data = { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| """Registration metadata: what an agent reports about itself at startup.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import pytest | ||
|
|
||
| from agentex.lib.utils.registration import build_registration_metadata | ||
| from agentex.lib.environment_variables import EnvironmentVariables | ||
|
|
||
| SHA = "b362b171a9c4e1f09d8e7a6b5c4d3e2f1a0b9c8d" | ||
|
|
||
|
|
||
| def _env(**overrides) -> EnvironmentVariables: | ||
| return EnvironmentVariables(AGENT_NAME="sample-agent", ACP_URL="http://agent", **overrides) | ||
|
|
||
|
|
||
| def test_nothing_known_yields_empty_metadata(): | ||
| assert build_registration_metadata(_env()) == {} | ||
|
|
||
|
|
||
| def test_commit_and_repo_reported_when_set(): | ||
| env = _env(AGENT_COMMIT_SHA=SHA, AGENT_SOURCE_REPO="git@github.com:scaleapi/Demo.git") | ||
| assert build_registration_metadata(env) == { | ||
| "commit_sha": SHA, | ||
| "source_repo": "github.com/scaleapi/Demo", | ||
| } | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("value", ["latest", "v1.2.3", "rocket_mock_agent-" + SHA, "abc", " "]) | ||
| def test_non_commit_values_are_omitted_not_forwarded(value): | ||
| """A field named for a commit never holds an image tag, same rule as __commit_sha__.""" | ||
| assert "commit_sha" not in build_registration_metadata(_env(AGENT_COMMIT_SHA=value)) | ||
|
|
||
|
|
||
| def test_repo_normalization_strips_scheme_and_credentials(): | ||
| env = _env(AGENT_SOURCE_REPO="https://x-token:secret@GitHub.com/scaleapi/Demo.git") | ||
| assert build_registration_metadata(env)["source_repo"] == "github.com/scaleapi/Demo" | ||
|
|
||
|
|
||
| def test_deployment_id_and_agent_card_still_reported(): | ||
| class Card: | ||
| def model_dump(self): | ||
| return {"name": "sample"} | ||
|
|
||
| env = _env(AGENTEX_DEPLOYMENT_ID="dep-1") | ||
| assert build_registration_metadata(env, Card()) == { | ||
| "deployment_id": "dep-1", | ||
| "agent_card": {"name": "sample"}, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new unconditional accesses to
AGENT_COMMIT_SHAandAGENT_SOURCE_REPObreak the existingregister_agenttests. Their environment stub intests/lib/test_agent_card.pydefines neither attribute, so all three tests raiseAttributeErrorbefore making their mocked HTTP requests. Update the fixture or read these optional fields defensively so the test suite can pass.Prompt To Fix With AI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in ee05394 by extending the
EnvVarsstub intest_agent_card.pywith the two new fields. The accesses stay unconditional on purpose:EnvironmentVariablesdeclares both fields, and a defensivegetattrwould hide a real typo in the model.🤖 — posted via Claude Code