Skip to content

Commit b609e24

Browse files
committed
address second-round review feedback on #952
- service-discovery-types(design): drop the dangling reference to ~/DiscoveryNotes/ files that aren't in the repo - service-matcher(matcher-vat): restore evicted entries on bridge ingest failure; the prior code rolled back the new entry but not the entries it had just superseded, so a transient bridge failure silently destroyed whatever previous registration shared the new one's providerTag - service-matcher(matcher-vat): drop the dangling reference to discovery-plan.md (which was removed earlier); inline the relevant obligations directly in the file header - sample-services(matcher-registration): change registerServicesWithMatcher to return a RegistrationSummary, throw on matcher-URL redeem failure, and throw when every entry fails — so bootstrap fails loudly instead of returning success with an empty matcher registry. Update call sites in echo-service and random-number-service to await the call instead of fire-and-forget, so the new throws actually surface
1 parent 9f0e86d commit b609e24

5 files changed

Lines changed: 62 additions & 27 deletions

File tree

packages/sample-services/src/echo-service/index.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,10 @@ export function buildRootObject(
6060
});
6161
contactUrl = await E(services.ocapURLIssuerService).issue(contact);
6262

63-
registerServicesWithMatcher({
63+
await registerServicesWithMatcher({
6464
matcherUrl,
6565
ocapURLRedemptionService: services.ocapURLRedemptionService,
6666
entries: [{ name: SERVICE_NAME, contact, registrationToken }],
67-
}).catch((error: unknown) => {
68-
// eslint-disable-next-line no-console
69-
console.error(`[${SERVICE_NAME}] Matcher registration failed:`, error);
7067
});
7168

7269
return harden({ name: SERVICE_NAME, contactUrl });

packages/sample-services/src/random-number-service/index.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,10 @@ export function buildRootObject(
6363
});
6464
contactUrl = await E(services.ocapURLIssuerService).issue(contact);
6565

66-
registerServicesWithMatcher({
66+
await registerServicesWithMatcher({
6767
matcherUrl,
6868
ocapURLRedemptionService: services.ocapURLRedemptionService,
6969
entries: [{ name: SERVICE_NAME, contact, registrationToken }],
70-
}).catch((error: unknown) => {
71-
// eslint-disable-next-line no-console
72-
console.error(`[${SERVICE_NAME}] Matcher registration failed:`, error);
7370
});
7471

7572
return harden({ name: SERVICE_NAME, contactUrl });

packages/sample-services/src/vat-lib/matcher-registration.ts

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,36 +14,53 @@ export type RegistrationEntry = {
1414
registrationToken: string;
1515
};
1616

17+
/**
18+
* Summary of a `registerServicesWithMatcher` call: which entries
19+
* landed in the matcher's registry, and which did not.
20+
*/
21+
export type RegistrationSummary = {
22+
registered: { name: string }[];
23+
failed: { name: string; cause: unknown }[];
24+
};
25+
1726
/**
1827
* Register a service (or group of services) with a matcher.
1928
*
20-
* If `matcherUrl` is empty/undefined, logs and returns without contacting
21-
* any matcher — useful for development cycles where the matcher isn't
22-
* up yet. Otherwise redeems `matcherUrl` via the kernel's redemption
23-
* service and calls `registerServiceByRef(contact, token)` for each
24-
* entry sequentially. Registration failures are logged but do not abort
25-
* the remaining registrations.
29+
* If `matcherUrl` is empty/undefined, logs and returns an empty summary
30+
* without contacting any matcher — useful for development cycles where
31+
* the matcher isn't up yet. Otherwise redeems `matcherUrl` via the
32+
* kernel's redemption service and calls
33+
* `registerServiceByRef(contact, token)` for each entry sequentially.
34+
* Per-entry failures are logged but do not abort the remaining
35+
* registrations.
36+
*
37+
* Throws when the matcher URL itself cannot be redeemed (there's no
38+
* useful partial-success story past that point) and when every entry
39+
* fails (so bootstrap returning success while the matcher's registry
40+
* stays empty becomes a loud failure instead of a silent one).
2641
*
2742
* @param options - Registration options.
2843
* @param options.matcherUrl - Ocap URL of the matcher, or empty/undefined
2944
* to skip registration entirely.
3045
* @param options.ocapURLRedemptionService - Kernel service used to redeem
3146
* the matcher URL.
3247
* @param options.entries - Services to register.
48+
* @returns Summary of which entries succeeded vs failed.
3349
*/
3450
export async function registerServicesWithMatcher(options: {
3551
matcherUrl: string | undefined;
3652
ocapURLRedemptionService: OcapURLRedemptionService;
3753
entries: RegistrationEntry[];
38-
}): Promise<void> {
54+
}): Promise<RegistrationSummary> {
3955
const { matcherUrl, ocapURLRedemptionService, entries } = options;
56+
const summary: RegistrationSummary = { registered: [], failed: [] };
4057

4158
if (!matcherUrl) {
4259
// eslint-disable-next-line no-console
4360
console.log(
4461
'[vat] matcherUrl parameter not set; skipping matcher registration.',
4562
);
46-
return;
63+
return summary;
4764
}
4865

4966
let matcher: ServiceMatcher;
@@ -52,9 +69,7 @@ export async function registerServicesWithMatcher(options: {
5269
matcherUrl,
5370
)) as ServiceMatcher;
5471
} catch (cause) {
55-
// eslint-disable-next-line no-console
56-
console.error(`[vat] Failed to redeem matcher URL ${matcherUrl}:`, cause);
57-
return;
72+
throw new Error(`Failed to redeem matcher URL ${matcherUrl}`, { cause });
5873
}
5974

6075
for (const entry of entries) {
@@ -63,11 +78,21 @@ export async function registerServicesWithMatcher(options: {
6378
entry.contact,
6479
entry.registrationToken,
6580
);
81+
summary.registered.push({ name: entry.name });
6682
// eslint-disable-next-line no-console
6783
console.log(`[vat] Registered service "${entry.name}" with matcher.`);
6884
} catch (cause) {
85+
summary.failed.push({ name: entry.name, cause });
6986
// eslint-disable-next-line no-console
7087
console.error(`[vat] Failed to register "${entry.name}":`, cause);
7188
}
7289
}
90+
91+
if (entries.length > 0 && summary.registered.length === 0) {
92+
throw new Error(
93+
`All ${entries.length} registration(s) failed for matcher ${matcherUrl}`,
94+
);
95+
}
96+
97+
return summary;
7398
}

packages/service-discovery-types/docs/design.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
# Service discovery — design notes
22

3-
Design notes for the types in this package, to be read alongside
4-
`~/DiscoveryNotes/discovery.md` (the background essay) and
5-
`~/DiscoveryNotes/discovery-analysis.md` (the analysis of the prior demo).
3+
Design notes for the types in this package.
64

75
This package only holds wire-format types, runtime validators, and a small
86
converter. The actual provider, matcher, and consumer live in their own

packages/service-matcher/src/matcher-vat/index.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@
1111
*
1212
* The registry of registered services is **in-memory**. After a
1313
* matcher restart, providers must re-register. Making the registry
14-
* durable is a planned follow-up — see the "dedup / liveness" entry
15-
* in `discovery-plan.md` for the design obligations that come with
16-
* that path.
14+
* durable is a planned follow-up; doing so brings its own obligations
15+
* (eviction of stale registrations when providers disappear, liveness
16+
* detection, and reconciling persisted entries with the bridge's LLM
17+
* context) that need to be designed before that change lands.
1718
*
1819
* Ranking is delegated to an LLM-backed bridge process via an
1920
* `IOService` endowment named `llm`. On every successful registration
@@ -128,7 +129,8 @@ export function buildRootObject(
128129
}
129130

130131
// Registry is in-memory. On matcher restart it starts empty;
131-
// providers must re-register. (See plan follow-up "dedup / liveness".)
132+
// providers must re-register. Making this durable is a planned
133+
// follow-up; see the file header for the obligations it carries.
132134
const registry = new Map<string, RegisteredService>();
133135

134136
const log = (...args: unknown[]): void => {
@@ -458,7 +460,17 @@ export function buildRootObject(
458460
description: ServiceDescription,
459461
contact: ContactPoint,
460462
): Promise<void> {
463+
// Stash superseded entries so we can restore them if the bridge
464+
// ingest fails — otherwise a transient bridge failure would
465+
// silently destroy whatever previous registration shared this
466+
// providerTag, defeating the atomicity property the dedup commit
467+
// was supposed to provide.
468+
const evicted: [string, RegisteredService][] = [];
461469
for (const supersededId of findSamePeerSameTagEntries(description)) {
470+
const prior = registry.get(supersededId);
471+
if (prior) {
472+
evicted.push([supersededId, prior]);
473+
}
462474
registry.delete(supersededId);
463475
log(
464476
`evicted superseded registration ${supersededId} (providerTag=${description.providerTag})`,
@@ -469,7 +481,13 @@ export function buildRootObject(
469481
await ingestService(id, description);
470482
} catch (cause) {
471483
registry.delete(id);
472-
log(`bridge ingest failed for ${id}; removed from registry:`, cause);
484+
for (const [oldId, oldEntry] of evicted) {
485+
registry.set(oldId, oldEntry);
486+
}
487+
log(
488+
`bridge ingest failed for ${id}; rolled back new entry and restored ${evicted.length} evicted entries:`,
489+
cause,
490+
);
473491
throw cause;
474492
}
475493
}

0 commit comments

Comments
 (0)