[1/5] Share authoring identity and deployed-agent discovery by rebova-microsoft · Pull Request #269 · microsoft/Employee-Self-Service-Agent-Developer-Kit

rebova-microsoft · 2026-09-11T08:18:09Z

Description

Extract a neutral, read-only deployed-agent discovery client and delegate the existing landing-page lookup tools to it. Expose shared tenant-local account matching and privacy-safe credential checks for the subsequent announcement slices, without adding another MCP process.

Current diff

Retain the existing account-preserving refresh behavior and normalize credential/cache failures without exposing private details.
Add isolated shared-client and discovery regression coverage while retaining the broader upstream cache and refresh suites.
Allow PR targets only for the announcements prerelease branch and its review-stack pattern in addition to existing main/release targets. Preserve existing push, PR-event, and security policy.

Feature contract

Discovery returns deployed agent identifiers without initializing feature configuration. Tenant/account context comes from authentication rather than tool arguments.

Stack and dependency

This is slice 1/5. Head: users/rebova/org-announcements-review-core. Base: users/rebova/org-announcements-prerelease. The current diff is only this slice against its immediate predecessor; these are stacked review chunks, not parallel PRs against the integration branch.

Bootstrap prerequisite: #262 merged into release/planner-landing-page on September 10, 2026. This stack remains pinned to cacb1bec056428809f1ccb0383561190d516bee4, which is an ancestor of merge commit 8a04f5f40f334e729a3497877edca655730f1be2. Unchanged prerequisite work is excluded from this slice. No rewrite is needed solely to account for that merge. The future release target remains TBD and its final promotion baseline must be confirmed separately.

Testing

293 Python tests passed from the clean committed slice; one inherited Windows-only cache test was skipped on macOS. Ruff 0.16.0 and syntax checks passed. No announcement runtime files were needed for this slice.

Local execution used Python 3.13.15. Python 3.11 GitHub Actions validation is pending; no local 3.11 pass is claimed.

Readiness

Live Graph, authoring-service, hosted-widget, and end-to-end integration checks are deferred and remain required before rollout.
This is not release approval and performs no deployment. The future release branch is still TBD.
The prerelease integration branch remains baseline-only until reviewed chunks are merged through the normal review process.
Relevant offline tests and static checks pass.
Python 3.11 CI results.
Authorized live integration and end-to-end validation.

Extract read-only deployed-agent discovery and reuse tenant-local account matching. Preserve account-bound token refresh and safe authentication failures. Add isolated regression coverage and narrowly scoped review-stack CI targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: eddd3818-bb74-42d3-bcf3-7e0670a57f27

rebova-microsoft · 2026-09-11T08:20:02Z

Review order: #269 (shared core/discovery) → #270 (contracts/client) → #271 (Graph audience) → #272 (MCP runtime) → #273 (maker/setup/packaging).

Each PR targets the preceding review branch; #269 targets users/rebova/org-announcements-prerelease. The prerelease branch remains at the shared baseline until the chunks are reviewed and merged. The complete local feature is at the #273 head.

Bootstrap PR #262 has merged, and the pinned base is in the release branch ancestry. Promotion to the future official release branch and main is separate. Paired widget/backend integration, live testing, and end-to-end validation remain outstanding; these draft PRs are not release approval.

rebova-microsoft · 2026-09-11T09:34:47Z

@microsoft-github-policy-service agree company="Microsoft"

Copilot

🟡 Changes recommended

Unreadable token files can still expose private filesystem paths, and the test importer suppresses unrelated warnings.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Extracts reusable deployed-agent discovery and authoring identity handling while preserving landing-page behavior and enabling stacked-review CI targets.

Changes:

Adds shared discovery, identity validation, and privacy-focused credential refresh handling.
Adds isolated MCP imports and regression coverage.
Expands CI pull-request targets for the announcement review stack.

File summaries

File	Description
`.github/workflows/ci.yml`	Adds review-stack targets and auth tests.
`agentconfig_core/_odata.py`	Centralizes title ID validation.
`agentconfig_core/agent_discovery.py`	Adds shared read-only discovery client.
`agentconfig_core/base_client.py`	Adds identity matching and sanitized refresh errors.
`agentconfig_landing_page/client.py`	Delegates discovery to the shared client.
`tests/mcp/_mcp_modules.py`	Adds collision-free MCP test imports.
`tests/mcp/conftest.py`	Isolates tests from real credentials and networks.
`tests/mcp/agentconfig/test_client.py`	Uses the isolated importer.
`tests/mcp/agentconfig/test_server_contract.py`	Uses the isolated importer.
`tests/mcp/agentconfig/test_widget_protocol.py`	Uses the isolated importer.
`tests/mcp/agentconfig_core/test_agent_discovery.py`	Covers shared discovery behavior.
`tests/mcp/agentconfig_core/test_base_client.py`	Expands identity and refresh coverage.
`tests/mcp/agentconfig_core/test_ci_contract.py`	Verifies CI trigger policy.
`tests/mcp/agentconfig_core/test_tenant_context.py`	Covers tenant discovery and dependencies.
`tests/mcp/agentconfig_core/test_token_cache.py`	Verifies private lock-file permissions.
`tests/mcp/agentconfig_core/test_token_refresh.py`	Verifies sanitized refresh failures.

Review details

Files reviewed: 16/16 changed files
Comments generated: 2
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

    with open(token_file, "r", encoding="utf-8") as handle:
        token = handle.read().strip()


+                    with warnings.catch_warnings():
+                        # Importing a FastMCP server transitively triggers
+                        # pydantic-settings' IncompleteFieldDefinitionWarning,
+                        # which the suite's ``filterwarnings = error`` would
+                        # otherwise turn into a collection failure. Suppressed
+                        # here, once, instead of at every call site.
+                        warnings.simplefilter("ignore")
+                        spec.loader.exec_module(module)


rebova-microsoft

File-by-file walkthrough

These 16 explanatory comments describe what each changed file does and why it belongs in this shared-foundation slice. They are reading aids, not an approval or a request for changes, and do not resolve the existing review feedback.

Scope: 4 runtime files, 1 CI file, and 11 test/support files. Of the 929 added lines, 774 are under tests/. This PR extracts shared discovery and identity primitives; it does not yet add the announcement provider or widget.

Suggested reading order: agent_discovery.py alongside the landing-page client deletions, then base_client.py with its regression cases, then the test-import support and CI changes.

rebova-microsoft · 2026-09-11T22:23:19Z

    return _validate_odata_string(value, name).replace("'", "''")


+def _validate_title_id(title_id: str) -> str:


Walkthrough — shared titleId validation.

This moves the existing landing-page title-ID rule into the shared helpers: reuse the established string validation and reject identifiers longer than 256 characters. titleId stays an opaque deployed-agent identifier, not a GUID requirement.

The purpose is one rule for both feature clients. This validator does not replace OData key escaping; callers still pass validated IDs through the existing encoding helper when constructing an agent URL.

rebova-microsoft · 2026-09-11T22:23:19Z

+    )
+
+
+class AgentDiscoveryClient(AgentConfigBaseClient):


Walkthrough — extract discovery once, without adding a server.

This new Python client contains the agent-list/search behavior moved out of the landing-page client, along with the existing payload-casing helpers and collection unwrapping. It inherits authentication, HTTP transport, and retries from AgentConfigBaseClient.

The operations remain GET tenants('{tenantId}')/EmployeeAgents and POST .../SearchAgents. Search trims its input and rejects empty or overlong strings. Responses may be a bare list or {value: [...]}; an unexpected shape raises an error rather than becoming an empty success.

The separate _agent_collection_path and explicit discovery conversion keep lookup independent of a feature's own collection route and canonical payload format. Landing-page consumes this shared code in this slice; announcements consumes it later. No third MCP process or configuration-initialization write is introduced.

rebova-microsoft · 2026-09-11T22:23:19Z

-
-
-class AgentConfigClient(AgentConfigBaseClient):
+class AgentConfigClient(AgentDiscoveryClient):


Walkthrough — the deleted discovery code moved to the shared layer.

AgentConfigClient now inherits from AgentDiscoveryClient, which itself inherits from AgentConfigBaseClient. This removes local copies of list/search, key-casing helpers, and title-ID validation; it does not remove those capabilities.

Landing-page-specific create/get/update operations stay here. The client retains its base URL and normal payload conversion, and _collection_path() delegates to the shared agent-collection path.

Read this deletion alongside agentconfig_core/agent_discovery.py: the intent is to preserve existing landing-page behavior while allowing a second feature provider to reuse discovery without calling the landing-page MCP process.

rebova-microsoft · 2026-09-11T22:23:19Z



-def _refresh_msal_token(tenant_id: str, object_id: str) -> str:
+def account_for_identity(app: Any, tenant_id: str, object_id: str) -> Any | None:


Walkthrough — reusable identity checks and controlled credential failures.

This is the main behavioral file in the slice; it is not a new login/token-cache implementation.

account_for_identity extracts the tenant-profile matching already used by token refresh. MSAL groups profiles by home account, so matching the tenant-local cached profile matters for identities such as guest accounts. The helper returns an account only when the match is unambiguous.

object_id exposes the account captured by the authoring client. validate_token_identity centralizes the tenant/account comparison so later resource clients can reuse it. Backend token validation and authorization remain authoritative.

Decoded token payloads must be JSON objects; malformed payloads are rejected explicitly.

Missing/empty token-file messages and MSAL failure messages stop echoing private paths or raw provider details. During refresh, validation, cache-lock, and filesystem failures become a controlled 401 AgentConfigApiError, retaining the original exception as its cause.

The account-preserving refresh mechanism was already upstream. This slice extracts its matching/checking logic and changes error handling. The separate existing review thread about an unreadable token file during initial construction remains open; this walkthrough does not claim that path is fixed.

rebova-microsoft · 2026-09-11T22:23:19Z

+    branches:
+      - "main"
+      - "release/**"
+      - "users/rebova/org-announcements-prerelease"


Walkthrough — run CI for this review stack, without opening every feature branch.

The PR-target filter adds exactly our prerelease branch and users/rebova/org-announcements-review-*. Without those entries, the stacked PRs would miss CI because they target each other rather than main or release/**.

The other change in this file adds the existing tests/scripts/test_auth.py to the foundation job because authentication is a shared dependency. Existing push filters, PR event types, and permissions are retained. The announcement-specific CI job is not part of this first slice; it arrives with the announcement modules in subsequent chunks.

rebova-microsoft · 2026-09-11T22:23:19Z

    )


+def test_the_sign_in_notice_goes_to_stderr_not_stdout(monkeypatch) -> None:


Walkthrough — why this is the largest added test block.

The expanded cases cover shared behavior both feature clients rely on: credential-source precedence, configured-tenant matching before a request, silent refresh of the original identity, rejected/mismatched replacement credentials, concurrent 401s sharing one refresh, privacy-safe surfaced failures, and checkout-anchored imports.

The sign-in tests also ensure human-readable browser notices go to stderr rather than corrupting MCP's JSON-RPC stdout stream. Tests use fake servers, accounts, tokens, and HTTP transports.

Much of this protects behavior already in the upstream foundation; the line count does not imply that this PR adds hundreds of lines of authentication runtime. The new assertions complement the account-helper extraction and error normalization in base_client.py.

rebova-microsoft · 2026-09-11T22:23:19Z

+import yaml
+
+
+def test_ci_accepts_only_the_approved_review_targets_in_addition_to_upstream():


Walkthrough — make the narrowly approved CI boundary explicit.

This test asserts the exact push and PR-target branch lists, the existing PR event types, and contents: read permissions. Its purpose is to catch accidental broadening of repository-wide CI policy while adding support for this particular stack.

It uses yaml.BaseLoader so the workflow key on remains a string rather than being interpreted as a YAML 1.1 boolean. These assertions are intentionally policy-specific; a deliberate future policy change should update them alongside the workflow.

rebova-microsoft · 2026-09-11T22:23:19Z

+
+
+@pytest.mark.parametrize("launch_folder", ["agentconfig_landing_page"])
+def test_discovers_configured_environment_from_feature_launch_directory(


Walkthrough — tenant discovery must work from a feature's launch directory.

This extends coverage for running from the landing-page MCP directory, parsing mocked Dataverse authentication challenges into a concrete tenant, and ensuring runtime installation paths include the shared dependencies.

The earlier configuration-path assertion is adjusted because the new automatic fixture deliberately redirects _CONFIG_PATH into a temporary location. The test still verifies the solution root and imported auth.py belong to this checkout, not a sibling worktree.

Only the landing-page launch directory is exercised in this first slice; future announcement files are not required. The tenant-discovery implementation and dependency declarations already exist in the upstream baseline and are not rewritten here.

rebova-microsoft · 2026-09-11T22:23:19Z


    assert stat.S_IMODE(directory.stat().st_mode) == 0o700
    assert stat.S_IMODE(path.stat().st_mode) == 0o600
+    assert stat.S_IMODE(Path(str(path) + ".lockfile").stat().st_mode) == 0o600


Walkthrough — one additional permissions assertion.

The existing POSIX permissions test already checks the cache directory (0700) and cache file (0600). This line extends the check to the synchronization lock file, also requiring 0600.

The token-cache implementation is unchanged in this PR. This is a small extension of the existing private-file coverage, not a new cache design.

rebova-microsoft · 2026-09-11T22:23:19Z

    async def run():
        try:
-            with pytest.raises(ValueError, match="does not exist"):
+            with pytest.raises(base_client.AgentConfigApiError) as caught:


Walkthrough — match the controlled refresh-error contract.

When the original token file disappears, refresh now surfaces a 401 AgentConfigApiError rather than a raw ValueError. The updated assertion checks the HTTP status, retains the underlying ValueError as the cause, and confirms the private file path is absent from the surfaced message.

The existing scenario still guards against silently switching from that original file to a different credential source. This test change follows the normalization in _refresh_token; it does not relax identity preservation or assert that initial token-file construction errors use the same path.

rebova-microsoft requested review from Asher Johnson Hogan (asherjh) and a balanced review from Copilot September 11, 2026 22:03

Copilot started reviewing on behalf of rebova-microsoft September 11, 2026 22:04 View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

rebova-microsoft commented Sep 11, 2026

View reviewed changes

rebova-microsoft marked this pull request as ready for review September 11, 2026 22:43

		with open(token_file, "r", encoding="utf-8") as handle:
		token = handle.read().strip()

		return _validate_odata_string(value, name).replace("'", "''")


		def _validate_title_id(title_id: str) -> str:



		class AgentConfigClient(AgentConfigBaseClient):
		class AgentConfigClient(AgentDiscoveryClient):



		def _refresh_msal_token(tenant_id: str, object_id: str) -> str:
		def account_for_identity(app: Any, tenant_id: str, object_id: str) -> Any \| None:

		)


		def test_the_sign_in_notice_goes_to_stderr_not_stdout(monkeypatch) -> None:

		import yaml


		def test_ci_accepts_only_the_approved_review_targets_in_addition_to_upstream():



		@pytest.mark.parametrize("launch_folder", ["agentconfig_landing_page"])
		def test_discovers_configured_environment_from_feature_launch_directory(

Uh oh!

Conversation

rebova-microsoft commented Sep 11, 2026

Description

Current diff

Feature contract

Stack and dependency

Testing

Readiness

Uh oh!

rebova-microsoft commented Sep 11, 2026

Uh oh!

rebova-microsoft commented Sep 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

rebova-microsoft left a comment

Choose a reason for hiding this comment

File-by-file walkthrough

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants