Skip to content

feat(studio): member-first endpoint contract + revision lifecycle APIs (#454)#457

Merged
eanzhao merged 4 commits intodevfrom
feat/2026-04-27_member-first-contract-revision-apis
Apr 27, 2026
Merged

feat(studio): member-first endpoint contract + revision lifecycle APIs (#454)#457
eanzhao merged 4 commits intodevfrom
feat/2026-04-27_member-first-contract-revision-apis

Conversation

@eanzhao
Copy link
Copy Markdown
Contributor

@eanzhao eanzhao commented Apr 27, 2026

Closes #454.

Summary

  • Adds three member-first HTTP routes the Studio frontend needs to finish removing serviceId from its Bind / Invoke / Observe paths:
    • GET /api/scopes/{scopeId}/members/{memberId}/endpoints/{endpointId}/contract
    • POST /api/scopes/{scopeId}/members/{memberId}/binding/revisions/{revisionId}:activate
    • POST /api/scopes/{scopeId}/members/{memberId}/binding/revisions/{revisionId}:retire
  • All three resolve the member-owned publishedServiceId internally through the existing StudioMember authority (IStudioMemberQueryPort), then call platform IServiceLifecycleQueryPort / IServiceCommandPort against the resolved identity. serviceId is never accepted as a user-facing input.

Why route through Studio (not platform's resolver)

Studio's BindAsync (PR #428) persists publishedServiceId = "member-{memberId}" via the StudioMember authority (StudioMemberConventions.BuildPublishedServiceId). Going through IStudioMemberQueryPort guarantees the new routes read the exact identity Studio's bind wrote, regardless of DefaultMemberPublishedServiceResolver's deterministic-id convention.

Response shapes (acceptance: stay stable with legacy where possible)

  • StudioMemberEndpointContractResponse mirrors ScopeServiceEndpointContractHttpResponse field-for-field; adds MemberId, sets InvokePath to the member-first URL.
  • StudioMemberBindingActivationResponse / StudioMemberBindingRevisionActionResponse mirror the legacy activation / revision-action payloads, with MemberId + PublishedServiceId for migration parity.

Architecture notes

  • StudioMemberService constructor adds IServiceLifecycleQueryPort + IServiceCommandPort. DI is unchanged for callers because both are already registered by the platform capability bundle.
  • Endpoint handlers depend only on IStudioMemberService (same pattern as the existing member-first endpoints).
  • Service-level error mapping is consistent across the surface: missing member → typed 404 STUDIO_MEMBER_NOT_FOUND; member exists but unbound / missing revision / retired-revision-activated → 400 with a stable code; endpoint-not-on-service → 404 STUDIO_MEMBER_ENDPOINT_CONTRACT_NOT_FOUND.

Acceptance criteria from #454

  • Backend provides a member-first endpoint contract route.
  • Backend provides member-first revision lifecycle routes (activate + retire).
  • All routes resolve the member-owned published service internally.
  • Response payloads stay stable with the existing service-route shapes where possible.

Test plan

  • dotnet build aevatar.slnx --nologo — 0 errors.
  • dotnet test test/Aevatar.Studio.Tests/Aevatar.Studio.Tests.csproj — 314 passed (was 290; +24 new for [Blocker] Missing member contract and revision lifecycle APIs block final Studio de-serviceId cleanup #454 + the existing-test constructor updates).
  • dotnet test test/Aevatar.GAgentService.Integration.Tests/... — 281 passed (no regressions; the platform side is untouched).
  • bash tools/ci/workflow_binding_boundary_guard.sh
  • bash tools/ci/query_projection_priming_guard.sh
  • bash tools/ci/projection_state_version_guard.sh
  • bash tools/ci/projection_state_mirror_current_state_guard.sh
  • bash tools/ci/projection_route_mapping_guard.sh
  • bash tools/ci/test_stability_guards.sh
  • bash tools/ci/cqrs_eventsourcing_boundary_guard.sh
  • bash tools/ci/committed_state_projection_guard.sh
  • bash tools/ci/solution_split_guards.sh
  • Architecture guards progress through every section before the slow playground_asset_drift_guard (which requires pnpm exec vite build and is not impacted by this PR — no playground/CLI assets touched).

Non-goals

  • No frontend work in this PR.
  • Existing scope-default + service-id routes (/binding/revisions/..., /services/{serviceId}/...) stay untouched for legacy callers.
  • The platform-side IMemberPublishedServiceResolver convention divergence is out of scope.

🤖 Generated with Claude Code

#454)

Closes #454.

Adds three member-first HTTP routes the Studio frontend needs to finish
removing serviceId from its Bind/Invoke/Observe paths:

- GET /api/scopes/{scopeId}/members/{memberId}/endpoints/{endpointId}/contract
- POST /api/scopes/{scopeId}/members/{memberId}/binding/revisions/{revisionId}:activate
- POST /api/scopes/{scopeId}/members/{memberId}/binding/revisions/{revisionId}:retire

All three resolve the member-owned publishedServiceId internally through
the existing StudioMember authority (IStudioMemberQueryPort), then call
platform IServiceLifecycleQueryPort / IServiceCommandPort against the
resolved identity. ServiceId is never accepted as a user-facing input.

The contract response mirrors the legacy ScopeServiceEndpointContractHttpResponse
shape so the frontend can keep its rendering, but exposes the member-first
InvokePath (/api/scopes/.../members/.../invoke/...). Activation/retire
responses include both memberId and publishedServiceId for parity with the
legacy serviceId-keyed shape during migration.

Why this routes through Studio (not platform's IMemberPublishedServiceResolver):
the Studio bind path persists publishedServiceId = "member-{memberId}" via
the StudioMember authority (StudioMemberConventions). Going through
StudioMemberQueryPort guarantees these new routes read the exact identity
Studio's bind wrote, regardless of the platform resolver's convention.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the member-first contract/revision API changes. I found one functional mismatch that should be addressed before relying on the returned invoke path.

return null;
}

private static string BuildMemberInvokePath(string scopeId, string memberId, string endpointId) =>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里返回的 InvokePath 会让前端调用现有 /members/{memberId}/invoke... handler,但那个 handler 仍通过 IMemberPublishedServiceResolver 解析成员服务;默认实现当前返回 PublishedServiceId = memberId,而 Studio bind/本方法解析到的是 member-{memberId}。结果是 contract 是按 member-{memberId} 构建的,实际 invoke 却会打到 {memberId} 服务,常见结果是 404 或调用错误服务。要么需要让 member invoke 路由同样走 IStudioMemberQueryPort/StudioMember authority,要么这里暂时不能返回现有 member invoke path。

@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

PR Review: Key Issues

1. [Medium] ~200 lines of contract-building logic duplicated from legacy

BuildMemberEndpointContractResponse and its helpers (ResolveCurrentContractRevision, ResolveStreamFrameFormat, EnumeratePreferredContractRevisionIds, BuildCurlExample, BuildFetchExample, BuildBase64PayloadPlaceholder) are near-identical clones of the legacy equivalents in ScopeServiceEndpoints.cs (BuildScopeServiceEndpointContractResponse, ResolveScopeServiceStreamFrameFormat, etc.).

This is the biggest maintainability risk in the PR. A bug fix in one copy will not propagate to the other. Consider extracting the shared contract-building logic into a shared utility that both the Studio Application layer and the legacy Hosting layer reference.

2. [Medium] Activate is non-transactional — SetDefault then Activate with no compensation

In ActivateBindingRevisionAsync, if ActivateServiceRevisionAsync fails after SetDefaultServingRevisionAsync succeeds, the system is left with the revision as default but not activated. No compensating action or rollback.

I see from the codebase this mirrors the legacy pattern in ScopeServiceEndpoints.cs, so this is inherited — but worth calling out explicitly in the method doc or a // NOTE: comment so future readers do not assume atomicity.

3. [Medium] Repeated resolve+verify pattern should be extracted

All three new methods share this opening pattern:

var (publishedServiceId, identity) = await ResolveMemberServiceIdentityAsync(scopeId, memberId, ct);
var service = await _serviceLifecycleQueryPort.GetServiceAsync(identity, ct)
    ?? throw BuildMemberNotBoundException(memberId);

Consider extracting ResolveBoundServiceContextAsync(scopeId, memberId, ct) returning (string publishedServiceId, ServiceIdentity identity, ServiceCatalogSnapshot service). This eliminates the repeated two-step query and makes the contract uniform across all three methods.

4. [Low-Medium] GetEndpointContractAsync makes 2 sequential queries where 1 may suffice

var service = await _serviceLifecycleQueryPort.GetServiceAsync(identity, ct) ...
var revisions = await _serviceLifecycleQueryPort.GetServiceRevisionsAsync(identity, ct);

ActivateBindingRevisionAsync calls GetServiceAsync + ResolveRevisionAsync (which internally calls GetServiceRevisionsAsync again), so activate makes 2 queries + 2 commands = 4 calls total. If the extracted ResolveBoundServiceContextAsync (issue #3) also fetches revisions, every path gets them in one shot.

5. [Low-Medium] Studio.Application depends directly on platform-level ports

StudioMemberService now has 5 constructor dependencies: 3 Studio-specific + 2 platform-level (IServiceLifecycleQueryPort, IServiceCommandPort). This means every test constructing StudioMemberService must stub all 5, even for bind-only tests. Consider introducing a thin IStudioMemberServiceLifecyclePort facade to keep the Application layer depending on its own abstractions. The test code already shows the pain — ThrowingServiceLifecycleQueryPort and ThrowingServiceCommandPort are 60+ lines of boilerplate just to assert "bind must not touch platform ports."

6. [Low] scopeId normalization inconsistency

ResolveMemberServiceIdentityAsync trims scopeId ((scopeId ?? string.Empty).Trim()), but the pre-existing methods (CreateAsync, ListAsync, BindAsync, etc.) pass scopeId through without trimming. Within the same service class, " foo " would be treated differently depending on which method you call.

7. [Low] Hardcoded Status: "retired" string in RetireBindingRevisionAsync

The response Status field is a bare string set to "retired". There is no const, enum, or shared vocabulary. If other lifecycle actions are added later, callers must guess the naming convention.

8. [Low] Input trimming at different depths

  • revisionId is trimmed immediately after validation (line 218, 267)
  • endpointId is only trimmed deep inside BuildMemberEndpointContractResponse (line 351)
  • scopeId is trimmed inside ResolveMemberServiceIdentityAsync (line 309)
  • Pre-existing methods do not trim any input

Pick one level (endpoint handler or service entry) and apply consistently.


Overall: The PR is well-structured with strong test coverage. The main concern is the duplicated contract-building logic (issue #1) — that is the highest-value refactoring target. The other issues are incremental improvements.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

❌ Patch coverage is 82.75862% with 75 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.25%. Comparing base (bafcd70) to head (28c1ce6).
⚠️ Report is 5 commits behind head on dev.

Files with missing lines Patch % Lines
...Application/Studio/Services/StudioMemberService.cs 80.76% 28 Missing and 17 partials ⚠️
...io.Application/Studio/Contracts/MemberContracts.cs 48.57% 18 Missing ⚠️
...stractions/Services/ServiceEndpointContractMath.cs 85.91% 3 Missing and 7 partials ⚠️
...vices/StudioAwareMemberPublishedServiceResolver.cs 93.93% 0 Missing and 2 partials ⚠️
@@            Coverage Diff             @@
##              dev     #457      +/-   ##
==========================================
+ Coverage   70.90%   71.25%   +0.35%     
==========================================
  Files        1209     1211       +2     
  Lines       87045    87420     +375     
  Branches    11411    11446      +35     
==========================================
+ Hits        61715    62295     +580     
+ Misses      20882    20646     -236     
- Partials     4448     4479      +31     
Flag Coverage Δ
ci 71.25% <82.75%> (+0.35%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...DependencyInjection/ServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
....Studio.Hosting/Endpoints/StudioMemberEndpoints.cs 86.95% <100.00%> (+8.90%) ⬆️
...Service.Hosting/Endpoints/ScopeServiceEndpoints.cs 81.86% <100.00%> (+<0.01%) ⬆️
...vices/StudioAwareMemberPublishedServiceResolver.cs 93.93% <93.93%> (ø)
...stractions/Services/ServiceEndpointContractMath.cs 85.91% <85.91%> (ø)
...io.Application/Studio/Contracts/MemberContracts.cs 74.28% <48.57%> (-12.86%) ⬇️
...Application/Studio/Services/StudioMemberService.cs 77.17% <80.76%> (+4.81%) ⬆️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ServiceId; share contract math

Addresses PR #457 review.

## Functional fix (the inline review): InvokePath / invoke handler mismatch

The contract returned by the new `GET /members/.../endpoints/.../contract`
was telling the frontend to call `/members/{memberId}/invoke/...`, but the
existing platform handler for that path resolves the member through
`IMemberPublishedServiceResolver` which today returns
`publishedServiceId == memberId`. Studio's bind path persists
`publishedServiceId == "member-{memberId}"`. So the contract was built for
`member-{memberId}` while invoke would target `{memberId}` → 404.

Fix: register `StudioAwareMemberPublishedServiceResolver` from Studio's
DI. It first asks `IStudioMemberQueryPort` for the member's stored
`publishedServiceId`; if no Studio member exists, falls back to the
legacy deterministic mapping (`memberId == publishedServiceId`) so
direct platform binds keep working unchanged. Now contract / activate /
retire / invoke / runs all resolve to the same identity.

## Refactors per the PR review

- **#1 Duplicated contract-building logic**: extracted the pure
  helpers (`ResolveCurrentContractRevision`,
  `EnumeratePreferredContractRevisionIds`, `RevisionContainsEndpoint`,
  `IsChatEndpoint`, `ResolveStreamFrameFormat`,
  `BuildBase64PayloadPlaceholder`, `BuildTypedInvokeRequestExampleBody`)
  into `Aevatar.GAgentService.Abstractions.Services.ServiceEndpointContractMath`.
  Both `ScopeServiceEndpoints.cs` (legacy) and `StudioMemberService.cs`
  (member-first) funnel through it. A bug fix in one helper now
  propagates to both paths automatically.

- **#3 / #4 Repeated resolve+verify pattern**: introduced
  `ResolveBoundServiceContextAsync` returning
  `(ScopeId, MemberId, PublishedServiceId, Identity, Service, Revisions)`.
  The three new methods now all share one query path; activate /
  retire dropped from 4 platform queries to 2.

- **#2 Non-atomic activate**: documented with a `NOTE:` comment that
  `SetDefaultServingRevision` then `ActivateServiceRevision` is
  intentionally non-transactional, mirroring the legacy scope-default
  behavior, and that both commands are platform-side idempotent.

- **#7 Hardcoded "retired" string**: introduced
  `MemberRevisionLifecycleStatusNames.Retired` next to the existing
  `MemberLifecycleStageNames` so future lifecycle verbs declare
  themselves alongside it instead of as scattered magic strings.

- **#6 / #8 Input trimming**: collapsed the four ad-hoc trimming sites
  into a single `NormalizeRequired(value, fieldName)` helper applied at
  the service entry of every public method. Trimming now happens at
  exactly one boundary per call.

## Tests

- 13 new tests pin the resolver's contract (Studio member → stored
  publishedServiceId; non-Studio member → legacy fallback; trim;
  reject malformed input; empty publishedServiceId degrades safely).
- Existing tests unchanged: 327 Studio + 281 platform integration
  passing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@eanzhao
Copy link
Copy Markdown
Contributor Author

eanzhao commented Apr 27, 2026

Follow-up Review: Fixes Assessment

All 8 issues from the first review have been addressed. Here is the assessment:

Fixed

# Issue Status How
1 ~200 lines duplicated contract logic Fixed Extracted ServiceEndpointContractMath into Aevatar.GAgentService.Abstractions/Services/. Both ScopeServiceEndpoints (legacy) and StudioMemberService now delegate to it. Legacy file has thin wrappers for call-site compat.
2 Non-transactional activate undocumented Fixed ActivateBindingRevisionAsync now has a clear // NOTE: block explaining the non-atomic two-command dispatch and idempotent retry convergence.
3 Repeated resolve+verify pattern Fixed Extracted ResolveBoundServiceContextAsync returning BoundServiceContext record struct. All three methods now use it uniformly.
4 2 sequential queries Fixed ResolveBoundServiceContextAsync bundles member authority + service catalog + revisions into one context object. Activate now makes 1 context resolution (3 internal queries) + 2 commands instead of the previous scattered 2+2 pattern.
7 Hardcoded "retired" string Fixed Introduced MemberRevisionLifecycleStatusNames with const string Retired = "retired".
8 Input trimming inconsistency Fixed Added NormalizeRequired helper in StudioMemberService. All three new methods normalize at entry.

Partially addressed

# Issue Status Notes
5 Studio.Application depends on platform ports Mitigated StudioAwareMemberPublishedServiceResolver was added to bridge the published-service-id gap for invoke routes — this is the most important new piece. The StudioMemberService constructor still takes 5 params, but the resolver eliminates the divergence risk that motivated the original concern. The boilerplate test stubs remain but are justified as regression guards.
6 scopeId normalization inconsistency Mostly fixed New methods normalize via NormalizeRequired. Pre-existing methods (CreateAsync, ListAsync, etc.) still pass through without trimming — but this is a pre-existing gap outside this PR scope.

New additions reviewed

StudioAwareMemberPublishedServiceResolver — Good design. Falls back to deterministic legacy mapping when the member is not Studio-managed. The services.Replace(...) registration in DI correctly wins over the platform TryAddSingleton. The DisallowedMemberIdChars validation mirrors the legacy resolver.

ServiceEndpointContractMath — Clean extraction. Lives in Abstractions (correct layer). Adds proper ArgumentNullException.ThrowIfNull / ArgumentException.ThrowIfNullOrWhiteSpace guards the original code lacked. The NormalizeOptionalNullIfEmpty rename is functionally equivalent but more descriptive.

Test coverageStudioAwareMemberPublishedServiceResolverTests (171 lines, 7 test cases) covers the key contract: Studio member → stored ID, non-Studio → fallback, empty publishedServiceId → fallback, input normalization, separator char rejection. Good.


Verdict: LGTM. All significant issues are resolved. The remaining minor items (pre-existing scopeId normalization gap, 5-param constructor) are acceptable trade-offs. Ship-ready.

Copy link
Copy Markdown
Contributor Author

@eanzhao eanzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the fix at 2a6b2a1. The resolver alignment itself is in the right direction, but host startup now exposes a blocking endpoint binding issue that needs to be fixed before merge.

string scopeId,
string memberId,
string endpointId,
IStudioMemberService memberService,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter needs an explicit [FromServices] (and the same applies to the other IStudioMemberService parameters in this file). When the app actually builds route endpoints, minimal API inspects IStudioMemberService and finds the instance method BindAsync(...); because it is not a valid static custom binder, host startup fails with BindAsync method found on IStudioMemberService with incorrect format. I reproduced it with dotnet test test/Aevatar.Hosting.Tests/Aevatar.Hosting.Tests.csproj --nologo --filter "FullyQualifiedName~MainnetHostCompositionTests|FullyQualifiedName~MainnetHealthEndpointsTests": both mainnet host startup tests fail before serving requests. Handler-only unit tests miss this because they call the methods directly instead of letting RequestDelegateFactory bind parameters.

eanzhao and others added 2 commits April 27, 2026 21:49
…ervices]

PR #457 follow-up: mainnet host startup fails with
"BindAsync method found on IStudioMemberService with incorrect format"
because Minimal API's RequestDelegateFactory probes every parameter
type for a BindAsync custom-binder hook, and IStudioMemberService
itself defines an instance method named BindAsync (the bind-revision
write path). Without [FromServices] on the parameter, the binder
matches the probe to that instance method, fails its shape check, and
the whole composition tears down before serving any request.

Reproducer (from the review):
    dotnet test test/Aevatar.Hosting.Tests/Aevatar.Hosting.Tests.csproj \
        --filter "FullyQualifiedName~MainnetHostCompositionTests"

Both MainnetHostCompositionTests and MainnetHealthEndpointsTests went
red on this. The handler-only unit tests in StudioMemberEndpointsTests
missed it because they call the static handlers via reflection and
never exercise RequestDelegateFactory.

Fix: decorate every IStudioMemberService parameter across all eight
handlers with [FromServices]. Class doc-comment now explicitly names
the failure mode so a future contributor doesn't strip the attribute
back off "for tidiness."

Regression guard: new StudioMemberEndpointsRouteBindingTests forces
endpoint construction (the exact codepath from the failing stack
trace) and asserts all 8 routes build. Without [FromServices] this
test goes red at the Studio test layer instead of mainnet startup.

Verified:
    test/Aevatar.Hosting.Tests   — 32 passed (was 30 + 2 failing)
    test/Aevatar.Studio.Tests    — 330 passed (+1 new regression test)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@eanzhao eanzhao merged commit df337cd into dev Apr 27, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Blocker] Missing member contract and revision lifecycle APIs block final Studio de-serviceId cleanup

1 participant