Repository navigation
Conversation
Add provider methods and types for the verification endpoints of
Cloud Registry 0.4.0 and later (GET /verifications, GET, PUT and
DELETE /services/{id}/verification), optional in the interface so
that custom providers still conform, and helpers that tell whether a
verification still matches its service, as the registry compares
them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add ecc-client-elixir-cloud-registry-service-verification: a badge for verified services, and "Verify" and "Remove verification" for admins. It sends the URL and API type of the service shown as the ones checked, takes a verification the app has loaded already (e.g. for a list, from getVerifications()), reacts to a provider set after the first update, and fires change and failure events with the status and conflict code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add code to CloudRegistryError, so that apps and the verification component read the conflict code of a 409 without digging into the problem. Test the component's busy state, races and failures of verify and remove, and link it from the package introduction. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A reload while a verify or remove ran (e.g. a new provider after a token refresh) dropped the result: no change event, the old state shown, and the button enabled again. Track loads apart from verify and remove, always report a result the registry has applied, and show it until the app sets verification again. Fold capital sharp s and dotless i as Python does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Track running verify and remove requests by service ID, so that one for a previous service doesn't block the service shown, and show a result (also one kept over the app's verification) only while the component shows the same service of the same registry (base URL). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Address automated review: load the verification again when the URL or API type of the service changes and after a verify or remove fails without a clear outcome (no status or a server error), and key running requests by registry and service, so that a request to one registry doesn't block another. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add ecc-client-elixir-cloud-registry-services, the services list of the service-registry package with a verification badge in each row. It loads all verifications with one request and follows verify and remove by admins. The elements it renders are defined only in browsers and only once, so that server-side rendering of the React wrappers keeps working. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ignore ecc-services-changed from a list of a previous provider, import the list's event types from the public events export, and test the remaining branches. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Reviewer's GuideData structuresDefine the verification data types and optional provider contract that the verification APIs and components depend on.
APIAdd REST provider methods for listing, reading, creating and deleting service verifications, and expose helpers that determine whether a verification still matches a service.
Define the typed change and failure events exposed by the verification components and re-export the service-list events they surface.
UIImplement the per-service verification badge and administrator actions, including loading, matching, event reporting and request-race handling.
Wrap the Service Registry list with a single verification-list request, matched row badges, propagated changes and failures, and app-provided row actions.
ConfigurationMake Service Registry services-element registration safe for server rendering and duplicate imports so the Cloud Registry wrapper can define it when needed.
Add TypeScript path mappings needed to resolve Service Registry component and event exports during the Cloud Registry build.
Record the Cloud Registry minor release and Service Registry patch release for the new components and guarded element registration.
DocumentationDocument the service-verification component, the verified services list, provider APIs and types, and links from the relevant package pages.
TestsTest verification REST calls, error-code handling and matching semantics, including the bulk helper's treatment of stale or unrelated verifications.
Test per-service verification rendering, administrator actions, failures and races, plus list loading, row actions, event propagation and stale-list handling.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Documentation build overview
50 files changed ·
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #581 +/- ##
==========================================
+ Coverage 75.31% 77.75% +2.43%
==========================================
Files 37 42 +5
Lines 9138 10207 +1069
Branches 470 605 +135
==========================================
+ Hits 6882 7936 +1054
- Misses 2245 2258 +13
- Partials 11 13 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
uniqueg
marked this pull request as ready for review
November 14, 2026 21:02
Contributor
Linked issuesChecked at commit 503e1af #579 feat: show the verified badge in the services list · 2 of 2 met
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 9 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts" line_range="132-138" />
<code_context>
+
+// Only in browsers, and only once (the services list of the
+// cloud-registry package may have defined it already)
+if (
+ typeof window !== "undefined" &&
+ window.customElements &&
</code_context>
<issue_to_address>
**Actions target the previous registry**
When the provider changes from A to B and B’s verification load has not completed or fails, `renderRowActions` keeps the closure created for provider A: it captures A and A’s verification map, and the new list for provider B receives it. While B’s verification request is pending—or if it fails—B’s rows show A’s badge state and admin actions send requests to A, potentially changing the wrong registry.
Recreate `renderRowActions` when the shown provider changes, so the new list cannot use the previous provider’s callback.
</issue_to_address>
### Comment 2
<location path="packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts" line_range="186-197" />
<code_context>
+ }
+
+ /** Updates the verifications after an admin verified or removed one. */
+ private handleVerificationChanged(
+ e: EccServiceVerificationChangedEvent
+ ): void {
+ if (!this.verifications) return;
+ const { serviceId, verification } = e.detail;
+ const others = this.verifications.filter(
+ (item) => item.serviceId !== serviceId
+ );
+ this.verifications = verification
+ ? [...others, { serviceId, ...verification }]
+ : others;
+ }
+
+ private handleViewDetails(serviceId: string): void {
</code_context>
<issue_to_address>
**Old actions corrupt the new registry**
When an admin’s verify or remove request is pending while the provider changes, `handleVerificationChanged` accepts the detached list’s event and adds its verification to the current list by service ID, so the new registry can show a false Verified badge.
Ignore verification-change events unless they came from the currently shown provider and list.
Also at `packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:241-253`.
</issue_to_address>
### Comment 3
<location path="packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts" line_range="251" />
<code_context>
+ .pageSize=${this.pageSize}
+ .rowActions=${this.renderRowActions}
+ @ecc-services-changed=${this.handleServicesChanged}
+ @ecc-service-verification-changed=${this.handleVerificationChanged}
+ ></ecc-client-ga4gh-service-registry-services>`
+ )}`;
</code_context>
<issue_to_address>
**Failed mutations leave stale badges**
When the registry applies a verify or remove but the response is lost or returns an error, the controlled `verification` prevents the verification component from reloading, and the list handles only successful change events, so the badge and available action remain out of sync with the registry.
Handle failed mutation events by reloading the service’s verification from the registry.
Also at `packages/ecc-client-elixir-cloud-registry/src/components/service-verification/service-verification.ts:322`, `packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:222`.
</issue_to_address>
### Comment 4
<location path="packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts" line_range="152" />
<code_context>
+ throw new Error("The provider doesn't support verifications.");
+ }
+ const verifications = await provider.getVerifications();
+ if (provider === this.shownProvider) this.verifications = verifications;
+ } catch (err) {
+ if (provider !== this.shownProvider) return;
</code_context>
<issue_to_address>
**Older requests overwrite current data**
When the app switches from provider A to B and back to the same A instance while A’s first list or verification request is pending, `loadVerifications` and `handleServicesChanged` accept results based only on provider object equality, so the first A request or event can overwrite newer services or verifications and leave the list stale.
Track the current provider request generation and ignore results or events from earlier generations.
Also at `packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:179-181`.
</issue_to_address>
### Comment 5
<location path="packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts" line_range="251" />
<code_context>
+ .pageSize=${this.pageSize}
+ .rowActions=${this.renderRowActions}
+ @ecc-services-changed=${this.handleServicesChanged}
+ @ecc-service-verification-changed=${this.handleVerificationChanged}
+ ></ecc-client-ga4gh-service-registry-services>`
+ )}`;
</code_context>
<issue_to_address>
**Off-screen changes are lost**
When an admin pages or filters a row out before its verification request completes, `handleVerificationChanged` never receives the success event if a row is filtered or paginated out before its verify or remove request completes: the event is dispatched from a detached row and cannot bubble to the list. The list keeps its old verification map, so returning to the row shows a stale badge and action.
Keep row mutations in list-level state that survives row removal, or reload verifications when a row is mounted again.
</issue_to_address>
### Comment 6
<location path="packages/ecc-client-elixir-cloud-registry/src/providers/rest-cr-provider.ts" line_range="215" />
<code_context>
+ */
+ async getVerifications(): Promise<ServiceVerification[]> {
+ const url = `${this.baseUrl}/verifications`;
+ const response = await this.fetch(url);
+ if (!response.ok) {
+ throw await CloudRegistryError.fromResponse(
</code_context>
<issue_to_address>
**Stalled requests never resolve**
When the registry or network leaves one of these requests pending rather than returning a response or rejecting, `getVerifications` and the verification mutation methods await `fetch` without a timeout or cancellation path. A stalled request leaves the list without badges and emits no load-failure event, or leaves an admin control busy indefinitely.
Add a bounded request timeout or cancellation mechanism and route timeout failures through the existing failure events while clearing pending state.
Also at `packages/ecc-client-elixir-cloud-registry/src/providers/rest-cr-provider.ts:261`.
</issue_to_address>
### Comment 7
<location path="packages/ecc-client-elixir-cloud-registry/src/components/service-verification/service-verification.ts" line_range="68-70" />
<code_context>
+ * Key of a running verify or remove: the service ID and the registry's base
+ * URL, if the provider has one
+ */
+const pendingKey = (id: string, provider: CloudRegistryProvider): string => {
+ const { baseUrl } = provider as { baseUrl?: unknown };
+ return JSON.stringify([typeof baseUrl === "string" ? baseUrl : "", id]);
+};
+
</code_context>
<issue_to_address>
**New provider actions stay disabled**
When a request for a service is pending while the component switches to a distinct custom provider without a string `baseUrl`, `pendingKey` maps both providers to the same key, so the new provider treats the old request as busy and keeps its Verify or Remove action disabled until that request settles.
Include a unique provider identity in the pending key when the provider has no string `baseUrl`.
</issue_to_address>
### Comment 8
<location path="packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts" line_range="88" />
<code_context>
+ @property({ type: Boolean }) admin = false;
+ @property({ type: Boolean }) filter = true;
+ @property({ type: Boolean }) search = true;
+ @property({ type: Number, attribute: "page-size" })
+ pageSize = DEFAULT_PAGE_SIZE;
+
</code_context>
<issue_to_address>
**Empty page size shows every service**
When the wrapper receives an empty `page-size` attribute, lit converts the empty attribute to `0`, and the inner list treats that value as “show all,” so callers see every service instead of the default page.
Use the default page size when the `page-size` attribute is empty.
</issue_to_address>
### Comment 9
<location path="packages/ecc-client-elixir-cloud-registry/src/components/services/tests/services.test.ts" line_range="207-208" />
<code_context>
+ );
+ await waitUntil(() => !!badge(el, "s1"), "verifications not shown");
+
+ expect(row(el, "s1")?.querySelector('[data-edit="s1"]')).to.not.equal(null);
+ expect(row(el, "s1")?.querySelector("[data-view-details]")).to.equal(null);
+ });
+
</code_context>
<issue_to_address>
**Actions before badge pass**
When the row renders the app's action before its verification badge, when the list renders app row actions before the verification badge, the test `renders the app's row actions after the badge` still passes: its assertions check that the app action exists and the default button is absent, not their order.
Assert that the badge precedes the app action in the row's rendered order.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 9 findings to address first, and a mistaken verification can persist in the registry and display a misleading “Verified” badge to users; reverting the component won’t remove that record. An admin can remove or correct the verification, so the effect is bounded and repairable.
Blocking findings: packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:138, packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:197, packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:251, packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:152, packages/ecc-client-elixir-cloud-registry/src/components/services/services.ts:251, and 4 more
Address Sourcery: number the lists so that answers and events of a replaced list are ignored, listen for verification changes on the badges so that rows off the page still count, reload the verifications after a failed verify or remove, and treat an empty page-size attribute as the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
@anuragxxd could you review #581? It shows the "Verified" badge in a services list, as a new component, It is stacked on #578; please review that one first. Only the last four commits belong to #581. Points for your decision:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Shows which services are verified in a services list, with one request for all verifications.
Stacked on #578 (#459, the verification component and provider methods): this branch contains #578's commits, so the diff shows them until #578 is merged. Only the last three commits belong to this PR: 544b764, 5537339, cdf3da2.
ecc-client-elixir-cloud-registry-servicesin the cloud-registry package. It wraps the services list of the service-registry package (ecc-client-ga4gh-service-registry-services). It loadsGET /verificationsonce per provider, matches the verifications to the services withverifiedServices(), and rendersecc-client-elixir-cloud-registry-service-verificationin each row through the list'srowActionshook (feat(service-registry): list filters and pagination #455/feat(service-registry): add list filters and pages #575). Each row gets itsverification(ornull), so rows send no requests of their own.adminis passed to the badges. The list appliesecc-service-verification-changedfrom admins' verify and remove without loading again.rowActionsrender after the badge. Without them, rows keep a "View Details" button that firesecc-service-selected.baseUrlchange creates a new inner list (keyed). Late answers from the previous provider, both verifications andecc-services-changed, are ignored.getVerifications(), the list shows no badges and firesecc-service-verification-failedwith an emptyserviceIdandaction: "load".index.tsfiles of the SR services list and the CR verification component now use the same guard, as the design package's files do. Without this, the docs build (Next.js SSR through the React wrappers) failed withwindow is not defined.tsconfig.jsongetspathsentries for@elixir-cloud/service-registry/components/*and/events, becausemoduleResolution: nodeignores packageexports.Docs: a new page
cloud-registry/components/services.mdx, a link in the package introduction, and updated callouts on the service-verification page and the SR services page. The verified filter that #455 mentioned isn't part of #575; this PR only shows the badge.Closes #579
Verification
npx web-test-runnerinpackages/ecc-client-elixir-cloud-registry: all tests pass in Chromium and Firefox, including 19 new tests for the component. Line coverage ofservices.tsis 100%. WebKit can't run on this machine (Playwright doesn't support Ubuntu 26.04); CI covers it.npx web-test-runnerinpackages/ecc-client-ga4gh-service-registry: 92 tests pass in Chromium and Firefox.npm run buildin both packages: passes, and generates the React wrapper@elixir-cloud/cloud-registry/react/services.npm run lintat the root (husky pre-commit, including the docs build): passes, with 0 errors.CI: all checks pass (Lint, Test on Node 22 and 24 including WebKit, CodeQL, Codecov patch and project, Read the Docs preview).
Review:
adversarial-reviewerandquality-reviewer, then a second adversarial round. Fixed:ecc-services-changedfrom a replaced list overwrote the services (medium);dist/path instead of the publiceventsexport (medium/low);baseUrlchange and failurestatus/cause(medium).Round 2 found nothing new.
Declined review findings
defineOncehelper (quality, low). A helper would touch every package's index files, which is out of scope. The guards copy the existing pattern from the design package.2.0.0-alpha.51(quality, low). This is the next release, the same version that feat(cloud-registry): verifications #578's page gives.ecc-service-selectedon it.Open questions
None. The design choice (a wrapper component in cloud-registry rather than a hook in the SR list) is open for the maintainer's review.
🤖 Generated with Claude Code
Summary by Sourcery
Add verified-service badges and administrator verification actions to the Cloud Registry services list.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: