Skip to content

[4/4] Compile-checked host-service exposure & versioning - #22

Open
alextomas955 wants to merge 14 commits into
v1.1-3-sdkfrom
v1.1-4-hostsvc
Open

[4/4] Compile-checked host-service exposure & versioning#22
alextomas955 wants to merge 14 commits into
v1.1-3-sdkfrom
v1.1-4-hostsvc

Conversation

@alextomas955

Copy link
Copy Markdown
Owner

Stack 4 of 4 (base: v1.1-3-sdk). Diff shows only this phase.

[ExposeToExtensions(typeof(IFoo))] + a Roslyn generator emits the DI forwarding and raises a compile-time error when a service doesn't implement the declared interface — retiring the manual interface+forwarding+smoke-test ritual (IMetadataServerService, IReferencePerformerImporter migrated). A single host contract version on system status; load-time compatibility negotiation that warns on dev builds and rejects on releases, without disabling already-installed extensions.

28 files, +977/−36. Version-incompatibility disable is in-memory only (re-evaluated each boot; no sticky disable across a host upgrade).

…tracts

- Add [ExposeToExtensions(typeof(IFoo))] attribute in Cove.Core.Contracts
- Add ServiceForwardingLifetime enum (Transient/Scoped/Singleton), infra-free
- Attribute carries InterfaceType and init-only Lifetime (defaults Transient)
- Add tools/host-services-gen netstandard2.0 Roslyn incremental generator
- Discover marked types via ForAttributeWithMetadataName on the attribute FQN
- Emit one AddCoveHostServices(IServiceCollection) with FQN-ordinal forwarding
- Report COVE0001 error when a marked type does not implement its interface
- Register the project in src/Cove.slnx
- Drive the generator with CSharpGeneratorDriver over in-memory sources
- Cover conforming transient emit on the metadata-server shape
- Cover singleton lifetime emit and fully-qualified ordinal ordering
- Cover COVE0001 error when a marked type does not implement its interface
- Add ContractVersion (semver-clean numeric) to SystemStatusDto alongside the display Version
- Populate it from the single CoveVersion source so extensions can negotiate min-host-version
- Disable an extension whose minimum host version exceeds this host's contract version on
  released builds, so it no longer initializes; surface an actionable message naming both the
  host version and the required floor
- Warn only (never disable) on development builds, detected by the -dev suffix on the display
  version plus the no-git 0.0.0 fallback, so local work against a pre-release host is not blocked
- Carry the full display version through ExtensionContext for that detection
- Add negotiation tests covering dev-warn, released-reject, the no-git fallback, satisfied live
  floors, and fail-closed handling of an unparseable floor
- Describe the host contract version and frontend runtime contract axes and which extensions pin
  which
- Cover @cove/types and @cove/extension-sdk versioning, the support window and deprecation notice,
  and how install-time and load-time negotiation surface to authors
- Reference the host-service registration generator as an analyzer in
  Cove.Api (OutputItemType=Analyzer, ReferenceOutputAssembly=false) so it
  runs at build time without emitting into output or downstream builds
- Mark MetadataServerService with [ExposeToExtensions(typeof(IMetadataServerService))]
- Mark ReferencePerformerImporter with the singleton-lifetime forwarding attribute
- Replace the two hand-written interface forwardings with a single
  AddCoveHostServices() call emitted from the marked concretes
- Preserve the typed AddHttpClient<MetadataServerService>() and a
  concrete AddSingleton<ReferencePerformerImporter>() registration
- Replace the single-service registration smoke test (now enforced at
  compile time) with a resolution test asserting both IMetadataServerService
  and IReferencePerformerImporter resolve to their host implementations
A host below an extension's minimum contract version previously persisted
enabled=false to the installation store. On the next boot the DB-wins reload
kept the extension disabled even after the host was upgraded to a compatible
version, so a floor that was later satisfied never re-enabled the extension.

Track the version-incompatibility skip in an in-memory set consulted by the
IsEnabled gate instead of mutating and persisting the Enabled flag. The floor
is now re-evaluated on every boot and the extension initializes normally once
the host satisfies it, while a deliberate user disable still persists.

Add regression tests covering the untouched persisted flag and re-enablement
after a host upgrade.
The syntax predicate matched only ClassDeclarationSyntax, so a record marked
for extension exposure was filtered out before analysis: it produced neither a
forwarding registration nor the interface-implementation diagnostic, surfacing
only as a runtime resolution failure. Records are permitted by the marker's
class target, so match RecordDeclarationSyntax as well.

Add generator tests for a conforming record and for a marked record that does
not implement its declared interface.
…e concrete types

When two concrete types (or one type carrying the attribute twice) are exposed
as the same interface, the generator emitted several registrations and DI kept
only the last, silently shadowing the others. Report COVE0002 at each offending
exposure so the ambiguity is caught at build time.

Add a generator test covering duplicate exposure of a shared interface.
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a Roslyn incremental source generator (host-services-gen) that replaces hand-written DI forwarding registrations with a compile-checked [ExposeToExtensions] attribute: the build now fails with COVE0001 if a marked type doesn't implement the declared interface, making it impossible to silently break the extension contract. Alongside this, it adds load-time host-contract-version negotiation that disables version-incompatible extensions in-memory on released builds (re-evaluated each boot) while only warning on dev builds.

  • MetadataServerService and ReferencePerformerImporter are migrated from manual AddTransient/AddSingleton forwarding registrations to [ExposeToExtensions]; the generated AddCoveHostServices() call in Program.cs replaces both.
  • SystemStatusDto gains a ContractVersion field (semver-clean major.minor.patch) distinct from the existing display Version; the OpenAPI contract, SDK types, and frontend .d.ts are all updated in sync.
  • Version-negotiation logic lives in ExtensionManager.EnforceDependencyCompatibilityAsync; the in-memory-only skip set (_versionSkippedExtensions) is carefully separated from the persisted Enabled flag so extensions re-activate automatically after a host upgrade with no operator action.

Confidence Score: 4/5

Safe to merge; the core DI migration and version-negotiation logic are well-tested and the in-memory-only disable design is sound.

The Location field in ExposedService will cause the incremental generator to skip its cache on every re-parse — a real but performance-only issue with no correctness impact. The IsDevelopmentBuild fallback behaviour is documented as using the numeric version when CoveVersionDisplay is unset, but the code instead falls through to release semantics in that case; the host always sets the field so this is dormant today, but it is a contract the API doesn't honour.

tools/host-services-gen/ExposedService.cs (Location equality) and src/Cove.Plugins/ExtensionManager.cs (IsDevelopmentBuild null-fallback) warrant a second look before the API surface grows.

Important Files Changed

Filename Overview
tools/host-services-gen/ExposedService.cs Record model for the incremental pipeline; Location field uses reference equality, defeating the incremental cache despite doc comment claiming correct caching.
tools/host-services-gen/HostServiceRegistrationGenerator.cs New incremental Roslyn source generator that emits AddCoveHostServices() with DI forwarding registrations and reports COVE0001/COVE0002 diagnostics; Location in ExposedService record breaks pipeline caching.
tools/host-services-gen/EquatableArray.cs Correct structural-equality ImmutableArray wrapper for the incremental pipeline; handles IsDefault gracefully in Equals and GetHashCode.
src/Cove.Plugins/ExtensionManager.cs Adds version-compatibility enforcement: released builds disable extensions below their min-host-version floor (in-memory only); dev builds warn. CoveVersionDisplay null-fallback documented but not implemented.
src/Cove.Core/Contracts/ExposeToExtensionsAttribute.cs New attribute and ServiceForwardingLifetime enum; clean, infrastructure-free contract in core assembly.
src/Cove.Api/Program.cs Replaces manual IMetadataServerService/IReferencePerformerImporter forwarding registrations with generated AddCoveHostServices(); adds CoveVersionDisplay to ExtensionContext.
src/Cove.Tests/VersionNegotiationTests.cs Comprehensive tests covering dev/release build detection, floor enforcement, in-memory-only disable, and re-enable after host upgrade.
src/Cove.Tests/HostServices/HostServiceRegistrationGeneratorTests.cs Generator unit tests using in-memory Roslyn compilation; covers conforming types, singleton lifetime, COVE0001 error, records, COVE0002 warning, and deterministic ordering.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Dev as Developer
    participant Roslyn as Roslyn Build
    participant Gen as HostServiceRegistrationGenerator
    participant DI as DI Container (Program.cs)
    participant EM as ExtensionManager
    participant Ext as Extension

    Dev->>Roslyn: Build (class annotated with [ExposeToExtensions])
    Roslyn->>Gen: ForAttributeWithMetadataName trigger
    Gen->>Gen: Extract() — check concrete implements interface
    alt implements interface
        Gen-->>Roslyn: ExposedService (conforming)
    else does not implement
        Gen-->>Roslyn: COVE0001 Error (build fails)
    end
    Gen->>Roslyn: Emit GeneratedHostServiceRegistrations.g.cs

    Note over DI: Startup
    DI->>DI: AddSingleton ReferencePerformerImporter
    DI->>DI: AddHttpClient MetadataServerService
    DI->>DI: AddCoveHostServices() — generated forwarding

    EM->>EM: EnforceDependencyCompatibilityAsync()
    EM->>EM: IsDevelopmentBuild() — check CoveVersion / CoveVersionDisplay
    alt "extension min-host-version > host version AND released build"
        EM->>EM: _versionSkippedExtensions.Add(id)
        EM-->>Ext: IsEnabled() returns false (this boot only)
    else dev build OR floor satisfied
        EM-->>Ext: IsEnabled() returns true
    end

    Ext->>DI: GetService IMetadataServerService
    DI-->>Ext: MetadataServerService instance
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Dev as Developer
    participant Roslyn as Roslyn Build
    participant Gen as HostServiceRegistrationGenerator
    participant DI as DI Container (Program.cs)
    participant EM as ExtensionManager
    participant Ext as Extension

    Dev->>Roslyn: Build (class annotated with [ExposeToExtensions])
    Roslyn->>Gen: ForAttributeWithMetadataName trigger
    Gen->>Gen: Extract() — check concrete implements interface
    alt implements interface
        Gen-->>Roslyn: ExposedService (conforming)
    else does not implement
        Gen-->>Roslyn: COVE0001 Error (build fails)
    end
    Gen->>Roslyn: Emit GeneratedHostServiceRegistrations.g.cs

    Note over DI: Startup
    DI->>DI: AddSingleton ReferencePerformerImporter
    DI->>DI: AddHttpClient MetadataServerService
    DI->>DI: AddCoveHostServices() — generated forwarding

    EM->>EM: EnforceDependencyCompatibilityAsync()
    EM->>EM: IsDevelopmentBuild() — check CoveVersion / CoveVersionDisplay
    alt "extension min-host-version > host version AND released build"
        EM->>EM: _versionSkippedExtensions.Add(id)
        EM-->>Ext: IsEnabled() returns false (this boot only)
    else dev build OR floor satisfied
        EM-->>Ext: IsEnabled() returns true
    end

    Ext->>DI: GetService IMetadataServerService
    DI-->>Ext: MetadataServerService instance
Loading

Comments Outside Diff (2)

  1. src/Cove.Plugins/ExtensionManager.cs, line 397-405 (link)

    P2 CoveVersionDisplay null falls through to release semantics, not the documented fallback

    The CoveVersionDisplay doc comment on ExtensionContext says "Optional; falls back to the numeric contract version when unset." The code here doesn't implement that fallback: when display is null (or empty/whitespace) and CoveVersion is not "0.0.0", the method returns false, treating the build as a release and enforcing version-incompatibility disabling. In the current host this is harmless because Program.cs always sets CoveVersionDisplay. However, any third-party or test consumer that creates ExtensionContext with only CoveVersion set to a dev-flavoured value would unexpectedly get release-build enforcement. The documented fallback should either be implemented — e.g., checking _context.CoveVersion when display is null — or the property should be documented as "caller must supply a value on dev builds."

  2. src/Cove.Plugins/ExtensionManager.cs, line 358-389 (link)

    P2 CancellationToken parameter is silently ignored

    EnforceDependencyCompatibilityAsync accepts ct but never passes it to anything — the method body is entirely synchronous and returns Task.CompletedTask. While harmless today, it creates misleading call-site expectations: callers that pass a linked CancellationToken get no cancellation semantics. Consider either renaming to a non-async signature or adding a brief comment noting that cancellation is accepted for API compatibility with the startup pipeline but the method does no I/O.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "chore(sdk): refresh rolled-up types for ..." | Re-trigger Greptile

Comment on lines +11 to +16
internal sealed record ExposedService(
string ConcreteFqn,
string InterfaceFqn,
ForwardingLifetime Lifetime,
bool Implements,
Location Location);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Location breaks incremental generator pipeline caching

ExposedService is a C# record, so the compiler generates equality by comparing every positional property. Location is a Roslyn reference type — two Location objects that describe the same source position are not equal unless they are literally the same object instance. Because the incremental pipeline uses equality to decide whether to re-run the Emit step, any incremental re-parse that creates a fresh Location for unchanged attributes will look like a change, defeating the cache and causing full regeneration on every keystroke. The doc comment says the record "caches correctly" because it holds no symbols or compilations, but Location is the remaining reference-equality type that breaks this. The fix is to either exclude Location from equality (override Equals/GetHashCode to ignore it) or store just the path+span as strings and reconstruct the Location in Emit only when needed.

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.

1 participant