From 046f5f15bd060e6074520152144ec29d2dd1d239 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Fri, 9 Oct 2026 18:51:03 -0500 Subject: [PATCH] fix(cave): read the familiar roster in Cave's contract spelling The discovered and managed transports parsed GET /api/client/v1/familiars with the earlier snake_case field names (`display_name`, `last_seen`, `active_sessions`). A real Cave has served the documented camelCase roster (`displayName`, `lastSeenAt`, `activeSessions`) since its canonical read projections landed, so `credentialStatus()` and `familiars()` refused every non-empty roster with `invalid_response` ("display_name must be a non-empty string"). A freshly exchanged credential therefore read as broken the moment the Cave had one familiar. The unit tests fed the parser an empty roster, which is how this escaped. Read the contract spelling first and keep the snake_case spelling as a fallback for an older Cave. The record still leaves the parser in the wire type's spelling, so the public types and `toFamiliar` are unchanged. `listFamiliars()` and the other canonical reads were already correct. Co-Authored-By: Claude Fable 5.1 --- .../cave-familiar-roster-contract-spelling.md | 13 +++ packages/cave/src/pairing.ts | 33 +++--- tests/cave-discovery-pairing.spec.ts | 104 ++++++++++++++++++ 3 files changed, 137 insertions(+), 13 deletions(-) create mode 100644 .changeset/cave-familiar-roster-contract-spelling.md diff --git a/.changeset/cave-familiar-roster-contract-spelling.md b/.changeset/cave-familiar-roster-contract-spelling.md new file mode 100644 index 00000000..fda1d4c7 --- /dev/null +++ b/.changeset/cave-familiar-roster-contract-spelling.md @@ -0,0 +1,13 @@ +--- +"@opencoven/cave-client": patch +--- + +Read the familiar roster in Cave's contract spelling. The discovered and +managed transports parsed `GET /api/client/v1/familiars` with the earlier +snake_case field names, while a real Cave serves the documented camelCase +roster (`displayName`, `lastSeenAt`, `activeSessions`), so `credentialStatus()` +and `familiars()` refused every non-empty roster with +`invalid_response` ("display_name must be a non-empty string"). The contract +spelling is read first; the snake_case spelling is still accepted for an older +Cave. `listFamiliars()` and the other canonical reads were already correct and +are unchanged. diff --git a/packages/cave/src/pairing.ts b/packages/cave/src/pairing.ts index df97687a..9a1007a2 100644 --- a/packages/cave/src/pairing.ts +++ b/packages/cave/src/pairing.ts @@ -421,10 +421,23 @@ function parsePairingExchange(value: unknown): CavePairingExchange { function parseFamiliar(value: unknown): CaveFamiliarWire { const familiar = expectObject(value, 'familiar'); + // Cave's reviewed contract spells the roster in camelCase (`displayName`, + // `lastSeenAt`, `activeSessions`; docs/api/client-v1.md), and a real Cave has + // served it that way since its canonical read projections landed. This + // parser only knew the earlier snake_case spelling, so `credentialStatus` + // and `familiars()` refused every non-empty roster with "display_name must + // be a non-empty string"; the unit tests never noticed because they fed it + // an empty roster. Read the contract spelling first and keep the snake_case + // one as a fallback for an older Cave. The record still leaves here in the + // wire type's spelling, and `toFamiliar` stays the one place it converts. + const displayName = familiar.displayName ?? familiar.display_name; + const lastSeen = familiar.lastSeenAt ?? familiar.last_seen; + const activeSessions = familiar.activeSessions ?? familiar.active_sessions; + const memoryFreshness = familiar.memoryFreshness ?? familiar.memory_freshness; return { id: expectString(familiar.id, 'familiar.id'), - display_name: expectString(familiar.display_name, 'familiar.display_name'), + display_name: expectString(displayName, 'familiar.displayName'), role: expectString(familiar.role, 'familiar.role'), ...(familiar.description === undefined ? {} @@ -435,24 +448,18 @@ function parseFamiliar(value: unknown): CaveFamiliarWire { ...(familiar.status === undefined ? {} : { status: expectString(familiar.status, 'familiar.status') }), - ...(familiar.last_seen === undefined + ...(lastSeen === undefined ? {} - : { last_seen: expectString(familiar.last_seen, 'familiar.last_seen') }), - ...(familiar.active_sessions === undefined + : { last_seen: expectString(lastSeen, 'familiar.lastSeenAt') }), + ...(activeSessions === undefined ? {} : { - active_sessions: expectTimestampNumber( - familiar.active_sessions, - 'familiar.active_sessions', - ), + active_sessions: expectTimestampNumber(activeSessions, 'familiar.activeSessions'), }), - ...(familiar.memory_freshness === undefined + ...(memoryFreshness === undefined ? {} : { - memory_freshness: expectString( - familiar.memory_freshness, - 'familiar.memory_freshness', - ), + memory_freshness: expectString(memoryFreshness, 'familiar.memoryFreshness'), }), }; } diff --git a/tests/cave-discovery-pairing.spec.ts b/tests/cave-discovery-pairing.spec.ts index 7eb4e97c..e4b5d93a 100644 --- a/tests/cave-discovery-pairing.spec.ts +++ b/tests/cave-discovery-pairing.spec.ts @@ -1354,6 +1354,110 @@ describe('discovered Cave pairing helpers', () => { }); }); + // A real Cave spells the roster the way docs/api/client-v1.md does, in + // camelCase. Before this test, `credentialStatus` and `familiars()` refused + // that roster outright ("display_name must be a non-empty string"), so a + // freshly exchanged credential read as broken the moment the Cave had a + // single familiar. The roster here is the documented sample. + test('reads the roster in the contract spelling after a discovered exchange', async () => { + const root = createScratchRoot('pairing-credential-status-contract-roster'); + await writeDiscoveryRecord(root, discoveryRecord()); + const contractRoster = [ + { + id: 'scribe', + displayName: 'Scribe', + role: 'Archivist', + description: 'Keeps the ledger.', + pronouns: 'they/them', + status: 'idle', + lastSeenAt: '2026-08-20T10:00:00.000Z', + activeSessions: 1, + }, + // The contract's optional fields are optional on the wire too. + { id: 'astra', displayName: 'Astra', role: 'Strategy / Navigation' }, + ]; + const fetchImplementation = queuedFetch([ + () => + jsonResponse( + 201, + successEnvelope({ + requestId: '018f4f1a-77c2-7a31-8a15-55a25aaba001', + secret: PAIRING_SECRET, + expiresAt: 1_755_731_112_617, + }), + ), + () => + jsonResponse( + 200, + successEnvelope({ + bearer: BEARER, + credential: { + id: '018f4f1a-77c2-7a31-8a15-55a25aaba002', + appName: 'OpenCoven Chat', + installationId: 'chat-install-1', + scopes: ['chat:read'], + createdAt: 1_755_730_812_617, + lastUsedAt: null, + revokedAt: null, + revocationReason: null, + }, + }), + ), + (_url, init) => { + expect(header(init, 'authorization')).toBe(`Bearer ${BEARER}`); + return jsonResponse(200, successEnvelope({ familiars: contractRoster })); + }, + (_url, init) => { + expect(header(init, 'authorization')).toBe(`Bearer ${BEARER}`); + return jsonResponse(200, successEnvelope({ familiars: contractRoster })); + }, + (_url, init) => { + expect(header(init, 'authorization')).toBe(`Bearer ${BEARER}`); + // Neither spelling carries a name: still a malformed roster. + return jsonResponse( + 200, + successEnvelope({ familiars: [{ id: 'nameless', role: 'Unknown' }] }), + ); + }, + ]); + const { client } = discoveredClient(root, fetchImplementation); + + const session = await client.createPairing({ + appName: 'OpenCoven Chat', + installationId: 'chat-install-1', + scopes: ['chat:read'], + }); + + await session.exchange(); + await expect(client.credentialStatus()).resolves.toEqual({ + status: 'valid', + access: 'chat:read', + health: caveHealth(), + }); + const familiars = await client.familiars(); + expect(familiars).toEqual([ + { + id: 'scribe', + displayName: 'Scribe', + role: 'Archivist', + description: 'Keeps the ledger.', + pronouns: 'they/them', + status: 'idle', + lastSeen: '2026-08-20T10:00:00.000Z', + activeSessions: 1, + }, + { id: 'astra', displayName: 'Astra', role: 'Strategy / Navigation' }, + ]); + // The wire spelling, in either casing, must not survive the client. + for (const familiar of familiars) { + expect(Object.keys(familiar)).not.toContain('display_name'); + expect(Object.keys(familiar)).not.toContain('lastSeenAt'); + } + await expect(client.familiars()).rejects.toMatchObject({ + code: 'invalid_response', + }); + }); + describe('discovered Cave canonical reads', () => { const routeInventories = [ {