[oznetmaster]

Move ITestAgentProvider into NUnit.Engine.Api so in-process, local-process and remote providers can share one public contract. Keep its NUnit.Engine.Services namespace alongside ITestAgentInfo, with IsAgentAvailable, GetAgent and ReturnAgent. No provider-discovery mechanism or remote-execution implementation is introduced here.

Following Charlie's and Manfred's reviews, use GetAgent / ReturnAgent and document the boundaries that callers need:

• Capability queries do not supply or reserve an agent. They may initialize provider resources, load or instantiate extensions, or contact another host. The provider remains responsible for those resources.
• GetAgent supplies a suitable agent; the caller must return the exact reference through its originating provider. Availability may change after a capability query. If acquisition throws, the provider remains responsible for resources created during that attempt.
• ReturnAgent returns the reference to that provider. After a successful return, the caller must stop using it. The caller need not know how agents are obtained or what resources the provider manages. No reuse or pooling policy is prescribed.
• A failed return does not guarantee that no work occurred. Preserve the originating-provider association rather than assuming retries are harmless. A successful return alone does not prove termination or that external resources are unused; safe resource cleanup is the responsible provider's concern for every agent type.

The latest follow-up intentionally renames ReleaseAgent to ReturnAgent on the new interface under review and revises its XML comments. This is not a documentation-only change. The other declarations, target frameworks and package dependencies are unchanged. It changes no existing released member; ITestAgentProvider is being added by this PR.

Integration and scope

After publication of an agreed API package containing this interface, the engine follow-up must remove its internal duplicate, consume that actual package and update provider/caller implementations to ReturnAgent together. This gives the engine and independently compiled providers one contract identity. It should retain the current coordinator/provider composition rather than revive the superseded #1917 implementation. No unpublished package version or private feed is committed here.

Provider construction/loading, remote transport and deployment, and the separate proposal to rename the engine coordinator TestAgency to TestAgentService are outside this API PR. Existing launcher and registration contracts are unchanged by this patch. The engine integration will need its own build and behavior validation on the coordinated public-package base.

Validation on Windows

• Complete API solution builds with upstream analyzers enabled: zero warnings/errors. Library targets are net462 and netstandard2.0.
• Existing API suites: 69 passed on net462 and 69 passed on net8.0; zero failures or skips.
• A separate compile-only provider/caller probe implements ITestAgentProvider and calls GetAgent/ReturnAgent on net462 and net8.0, referencing only the API project, with no engine implementation dependency. Both builds have zero warnings/errors. This is a contract-boundary check, not a runtime or lifecycle test.
• Both library targets generate well-formed XML for the interface and its three methods, including ReturnAgent with no stale ReleaseAgent reference.
• A declaration comparison confirms the only non-comment source change in this follow-up is the intended ReleaseAgent-to-ReturnAgent rename. Changed-file whitespace verification and git diff --check pass; the full-index patch independently reapplies to the verified candidate tree.

No fresh engine integration, Linux/macOS, GUI, adapter or hardware result is claimed for this API follow-up. Earlier prototype results belong to their original dependency graph.

Prepared by Neil Colvin with assistance from OpenAI Codex.

[CharliePoole]

Let's spell everything out in the interface. I'll send you some details about agent implementation that I referred to separately.

[CharliePoole]

I would remove the sentence "The engine loads..." It sounds as if this the path is an extension point path, while it's just the relative location of the source code and not needed here. The "Capability queries statement could be moved to IsAgentAvailable or left here as you prefer. However, it needs to be qualified, perhaps in a "Remarks" section, perhaps in a separate document to which we refer, perhaps both.

Agents can be written by anyone. There is a mechanism to enquire regarding certain capabilities without instantiating the engine, provided the author uses that mechanism. In the early V4 code, that mechanism didn't exist. It was added later and the four existing V4 agents make use of it. However, the engine doesn't require that it be used and falls back to loading the extension into memory and instantiating the engine if the necessary attributes are not provided.

I realize that the above is not an adequate explanation, but let's decide where such an explanation should live and I'll write one.

[CharliePoole]

Add something under InvalidOperationException to say you should check first using IsAgentAvailable I also think this needs a remark about ownership

[CharliePoole]

I like the detailed explanation of who is responsible for what. We may want to have more details somewhere, particularly regarding how agents work with respect to queries, i.e. sometimes needing instantiation, sometimes not. I think it's ready to merge but I'd like a second review since it's pretty important.

[CharliePoole]

@manfred-brands Even though the code is small, this interface and it's comments describe how we want the engine to acquire and interact with agents in the future. It was previously an internal interface used only for the single type of agent we supported up to now. We think this will work for all three types: in-process, local process and remote. It's due to the remote agents, supplied by a server, that we need to make this a public external interface. Because we now use multiple agent providers we have introduced the notion that each provider has ownership of the agents it supplies and needs to take care of any cleanup when they are released.

Bottom line, this is a design review as well as a code review. Neil and I did the design, so I think your separate review is a good idea in case we missed anything.

[manfred-brands]

@CharliePoole @oznetmaster

The idea and the described operation are fine.
I do have some remarks about (inconsistent) terminology.

[manfred-brands]

That is obvious as there is no return value to pass into ReleaseAgent.
I suggest to remove the first part.

Suggested change

	/// If acquisition throws, the caller has no returned agent to release. The provider
	/// If acquisition throws, the provider

[manfred-brands]

reconciling an outcome that cannot yet be confirmed.

Note sure what that means?

[CharliePoole]

@oznetmaster wrote

By an unconfirmed acquisition outcome I meant, for example, that the remote host creates an agent but its reply is lost. GetAgent can then fail even though an agent exists on the host. The provider must determine what was created and handle its ownership and cleanup; the caller cannot assume that the failed call created nothing. I will replace “reconciling an outcome” with that concrete explanation, including equivalent wording for a failed return.

I think any such concrete explanation should indicate that it applies to remote providers.

[manfred-brands]

Yes, I think that whole sentence can be left out. The first part is enough to say that the provider is responsible for cleaning up.

[manfred-brands]

We should be consistent in terminology and not change from acquired to provided.

Suggested change

	/// Returns a previously acquired test agent to the provider that supplied it.
	/// </summary>
	/// <param name="agent">An agent previously provided by a call to GetAgent.</param>
	/// Returns a previously acquired test agent to the provider that supplied it.
	/// </summary>
	/// <param name="agent">An agent previously acquired by a call to GetAgent.</param>

[manfred-brands]

I have no problem with Get but the help text on the Release method calls about acquired agent.
So maybe rename this to AcquireAgent?

[manfred-brands]

Terminology.
Release to me implies Disposal which is explicitly denied in the last paragraph.
But I might be biased by the IFactory in Castle.Windows we used before MS DI.

Would ReturnAgent be a better term?

The interface comments talks about returning an agent.
The summary says returns an agent
Later on it becomes releasing, but it is stated that the provider can return the same agent for another call and calling ReleaseAgent does not mean releasing resources.

This interface is like a pool of agents, MS uses Get/Return for ObjectPool and uses Rent/Return for ArrayPool

[CharliePoole]

I agree, let's go with ReturnAgent.

[CharliePoole]

Maybe drop the statement that the provider can return the same agent. Is there any reason the caller needs to know that?

[oznetmaster]

@manfred-brands @CharliePoole, thank you. I agree with simplifying the redundant sentence and consistently using “acquired” in the parameter documentation.

By an unconfirmed acquisition outcome I meant, for example, that the remote host creates an agent but its reply is lost. GetAgent can then fail even though an agent exists on the host. The provider must determine what was created and handle its ownership and cleanup; the caller cannot assume that the failed call created nothing. I will replace “reconciling an outcome” with that concrete explanation, including equivalent wording for a failed return.

On naming, I lean toward GetAgent/ReturnAgent. GetAgent can acquire an existing or newly created agent; ReturnAgent clearly ends the caller's right to use it while leaving the provider to decide whether to reuse it or stop it. This follows the Get/Return terminology you cited without requiring every provider to implement a pool. AcquireAgent/ReturnAgent would also work if the team prefers the more explicit acquisition name.

I suggest agreeing the method names here before changing the public contract and engine call sites together. I will keep the existing signatures until that is settled.

Prepared by Neil Colvin with assistance from OpenAI Codex.

[CharliePoole]

Let's go with GetAgent / ReturnAgent.

In the comment text, I'd like to retain the existing emphasis on the client not knowing whether a provider is a factory, a pool of agents or something else and not knowing what resources it manages. On the client side, we can talk about "getting," "acquiring" and "obtaining" without confusion but should use "return" consistently. We should always refer to the provider as a "provider" unless somebody can think of a word that's synonymous rather than having a more specific meaning as do "factory" and "pool." The provider "provides" agents or "returns" them. It may or may not create them, for example.

For reference purposes... we currently recognize three types of agents -- there could be more some day -- as follows...

• LocalProcess this provider is, in fact a factory for agents running on a separate local process. As in the past, agents are not reused but simply disposed upon return.
• InProcess this provider loads a single agent and will reuse it if possible.
• RemoteProcess this provider will use an interface we have yet to define to select remote servers. The servers will provide agents. There's no decision so far on reuse here.

General notes on reuse...

• Agents are tailored to specific packages, so any pool of agents would need to be indexed by key characteristics of the package. Defining "key" is the difficult part. There's an obvious tradeoff here, which depends on how costly it is to create an agent.
• An in-between option would be maintaining the process and wrapping it with a new agent. A similar examination of tradeoffs would apply.
• The immediate solution is pretty obvious... not to provide any reuse at all in the initial implementation and most likely in the initial release of 4.0.

@oznetmaster Is this enough for you to update? We'll then re-review and hopefully merge.

[CharliePoole]

Although not part of this PR, the talk of naming makes me think of this question...

I removed the (unused) TestAgentService and made TestAgency the top level provider. LocalTestAgentProvider now has most of the code that was previously in TestAgency. TestAgency now does what the deleted TestAgentService used to do... coordinate individual providers. It's basically a compound provider that functions as a service.

Maybe I am hanging on to the "agency" metaphor unnecessarily. Should we rename TestAgency to TestAgentService and be done with it?

[oznetmaster]

@CharliePoole @manfred-brands, yes, that is enough to update. I have prepared GetAgent/ReturnAgent with "return" used consistently. I removed the redundant acquisition sentence and the unclear outcome wording, leaving the provider's cleanup responsibility stated directly. I also removed the reuse/pool details so callers need not know how the provider manages agents or resources.

The revised API builds cleanly, its 69 existing tests pass on each runtime, and an independent provider/caller compiles against the new names on both targets. The engine follow-up will need to remove its internal duplicate and adopt ReturnAgent alongside the agreed published API package.

On the separate coordinator question, I favor TestAgentService: it describes its present role as a service composing providers. I would make that a separate engine refactor, retaining the current provider composition and launcher/registration contracts.

Prepared by Neil Colvin with assistance from OpenAI Codex.

[CharliePoole]

This looks ready to merge as far as I can see. Of course, it may change a few times before release due to our lack of omniscience. :-) Please wait for an OK from @manfred-brands as well before merging.

[manfred-brands]

Thanks @oznetmaster and @CharliePoole.

Looks good, descriptive and clear.