Conversation
Stage 1 Task 3 of the familiars integration plan (docs/superpowers/plans/2026-09-02-familiars-integration.md): add the two new Cave client-v1 reads Cave and the SDK already promoted today (coven-cave #5288, sdk #104/#105) to Chat's native command boundary. - src-tauri: two new commands, `cave_familiar_contract` and `cave_familiar_analytics`, mirroring the existing per-id read shape (`cave_get_conversation`). New `CaveReadPath::FamiliarContract`/ `::FamiliarAnalytics` variants build `/api/client/v1/familiars/:id/ {contract,analytics}`, with a `validate_canonical_familiar_id` bound matching Cave's own familiar-id charset and an analytics query builder bounded to Cave's `window`/`recent` parameters. New Rust unit tests cover id-segment encoding and out-of-bound window/recent rejection. Guard arrays (build.rs, capabilities/default.json, lib.rs, commands.rs) and the regenerated desktop schema stay in lockstep with specification-guards.test.ts. - TypeScript SDK boundary: `QueryAdapter.familiarContract`/ `familiarAnalytics` (cached like the other detail reads), `CaveReadClient` widened to the two new `CaveClient` methods, and the native transport implementation using the SDK's newly-exported `canonicalFamiliarContractData`/`canonicalFamiliarAnalyticsData` envelope helpers. - vendor/opencoven-sdk: re-packed `cave-client-0.1.0.tgz` and `sdk-core-0.1.0.tgz` from current `OpenCoven/sdk` main (commit d24b2ef), which already carries `familiarContract`/`familiarAnalytics` as unreleased changesets. Deliberately NOT included: `contract-canary.lock.json` and `phase1-conformance.lock.json` are not re-pinned to the new SDK/Cave revisions. Re-pinning requires the SDK repo's `create-release-artifacts.mjs`, which is gated by `release.config.json` `publishingEnabled: false`, and per `docs/workflows/client-v1-cross-repository-conformance.md` the whole chain is deliberately "no offline acceptance mode": a live, protected GitHub Actions environment (`client-v1-conformance`) with a named required reviewer. That re-pin needs to happen through the real protected workflow, not by hand here. Follow-up commit once that authority exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ueGDYGKGaMzNq5mTE6ogG
Stage 1 Task 4 of the familiars integration plan
(docs/superpowers/plans/2026-09-02-familiars-integration.md), built on top
of the previous commit's native contract/analytics reads. Against the mock
source only -- no shell wiring here (Task 5) and no dependency on the
contract-canary lock re-pin.
- src/familiars/source.ts: the `FamiliarsSource` interface and its Stage 1
view types (`FamiliarSummary`, `FamiliarDetail` with ward/identity/
report, `FamiliarActivity`, `ConversationSummary`, `ThreadMessage`) and
the `Capability` union from the integration design's data model and
capability-gating tables.
- src/familiars/mappers.ts (+ tests): the only module that knows the shape
of a `CaveCanonicalFamiliar`, `CaveFamiliarContract`, or
`CaveExecutionWindow` -- pure functions from SDK wire types to the view
types above, tested against literal SDK-fixture-shaped objects (renames,
optional-field omission, the window-not-served and null-completion-rate
cases).
- src/familiars/cave-source.ts (+ tests): `CaveFamiliarsSource`, a thin
fetch-then-map over `QueryAdapter` (already carrying `familiarContract`/
`familiarAnalytics` from the previous commit) -- no caching or retry
logic of its own, non-ok `QueryResult`s pass straight through unmapped.
- src/familiars/mock-source.ts (+ tests): `MockFamiliarsSource` wrapping
today's `src/demo/familiars-data.ts` and `mock-familiars.ts` into the
same view types. The demo's `FamActivity` is presentation-shaped
("100%", "1m 36s") because it matches the design mockup verbatim, so
reconstructing numeric fields from it is necessarily best-effort;
reasoning/hold/image/divider messages are dropped rather than faked,
since Cave does not serve rich content, attachments, or attention items
until Stage 3/4.
- src/familiars/capabilities.ts (+ tests): `availabilityFor(control,
capabilities)`, covering every row of the design's gating table.
Not included: wiring `FamiliarsShell` to either source (Task 5), or
`ChatShell` retirement (Task 6) -- those come once Task 3's lock re-pin
unblocks a real `CaveFamiliarsSource` in production.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ueGDYGKGaMzNq5mTE6ogG
Continues Task 5 of the familiars integration plan, scoped down after reading the shipped demo shell closely: `src/demo/familiars-shell.tsx` renders reasoning cards, held actions, image cards, @-mention guest replies, summoning, and a screen-watch overlay -- Stage 2-4 concepts with no Cave route yet -- plus fields (emoji, soul, memory) Cave's Stage 1 contract never serves. Reworking that component against `FamiliarsSource` today would mean either inventing data Cave doesn't send or silently downgrading the shipped, reviewed demo. Neither is honest, so this is a new, smaller `FamiliarsReadsShell` instead: sidebar, thread, and inspector driven entirely by a `FamiliarsSource`, rendering only Stage 1 reads (familiars, conversations, per-conversation text messages, ward/contract, analytics), with every other control -- send, mentions, held actions, reasoning, images, summon, screen -- showing a one-line "not available yet" notice from `availabilityFor` instead of a working mock. `src/demo/familiars-shell.tsx` is untouched; the existing rich `?demo=chat` experience is unaffected. Reachable at `?demo=familiars-reads` against `MockFamiliarsSource`, added to the route-preservation e2e smoke test. 7 new component tests cover the happy path, conversation switching, tab gating (both advertised and not), and a source that fails every read. `CaveFamiliarsSource` (previous commit) plugs into `FamiliarsReadsShell` today, mechanically -- switching `main.tsx` or `app.tsx` to it, and `ChatShell` retirement, still wait on the Task 3 lock re-pin so the first real render is against verified reads rather than an unverified one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016ueGDYGKGaMzNq5mTE6ogG
Splits the Cave-backed data path out of the Stage 1 work so the rest can land now. The vendored SDK tarballs are restored to the artifacts that contract-canary.lock.json pins, which the canary job checks out and verifies against the reviewed SDK revision. Bumping them needs a release manifest, and that needs the authorized release cut, so the vendor bump waits rather than turning main's canary red. What lands: the native familiar contract and analytics read paths, the data-source seam and its mock implementation, the capability additions, and the reads shell. All of it typechecks against the pinned SDK. What waits for the SDK bump: mappers.ts and cave-source.ts, plus the familiar transport in native-boundary.ts. Between them they need six symbols the pinned tarball does not export -- CaveFamiliarIdentity, CaveFamiliarWard, CaveExecutionDay, CaveFamiliarAnalyticsTransportOptions, canonicalFamiliarContractData, and canonicalFamiliarAnalyticsData. Nothing in the app imports either module yet, so removing them costs no wiring; the seam is shaped so the Cave-backed source arrives additively. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KacEJmkX5GhkViPUhx9Mie
There was a problem hiding this comment.
🟡 Changes recommended
A few newly introduced behaviors/docs are inconsistent (and the new query-adapter reads lack direct unit coverage), which should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR lands the “Stage 1 reads” slice of the Familiars integration against the currently pinned SDK by introducing a FamiliarsSource seam with a mock implementation, adding a dedicated reads-only shell, and wiring the corresponding native read commands/capabilities end-to-end (Rust → Tauri schema/capabilities → TS query adapter).
Changes:
- Add new native read paths/commands for familiar contract + analytics, plus capability/schema/guard updates to keep Tauri command registration in sync.
- Introduce
FamiliarsSource(Stage 1 reads) withMockFamiliarsSource, and a newFamiliarsReadsShellUI + CSS + unit tests. - Extend the TS SDK boundary (
QueryAdapter,CaveReadClient) withfamiliarContractandfamiliarAnalytics, and update affected tests/e2e routing.
File summaries
| File | Description |
|---|---|
| src/specification-guards.test.ts | Extends guard coverage to include new familiar read capabilities/commands. |
| src/main.tsx | Adds ?demo=familiars-reads route to render the new reads shell with a mock source. |
| src/lib/sdk/query-adapter.ts | Adds familiarContract/familiarAnalytics reads with cache keys and TTL handling. |
| src/lib/sdk/query-adapter.test.ts | Updates mock CaveReadClient shape for new methods. |
| src/lib/sdk/connection-controller.ts | Extends CaveReadClient pick to include familiar read methods. |
| src/familiars/source.ts | Adds Stage 1 FamiliarsSource interface + view-model types and capability list. |
| src/familiars/reads-shell.tsx | Implements a reads-only Familiars shell driven purely by FamiliarsSource. |
| src/familiars/reads-shell.test.tsx | Adds RTL tests for the reads shell behavior and capability notices. |
| src/familiars/reads-shell.css | Adds minimal styling for the reads shell. |
| src/familiars/mock-source.ts | Implements a mock FamiliarsSource backed by demo fixtures. |
| src/familiars/mock-source.test.ts | Adds unit tests for createMockFamiliarsSource mapping behavior. |
| src/familiars/capabilities.ts | Introduces per-control capability gating helper (availabilityFor). |
| src/familiars/capabilities.test.ts | Tests capability gating coverage and messaging behavior. |
| src/chat-shell.test.tsx | Updates query-adapter test double to include new methods. |
| src/app.test.tsx | Updates query-adapter test double to include new methods. |
| src-tauri/src/transport.rs | Adds familiar contract/analytics read paths, validation, query building, and Rust tests. |
| src-tauri/src/lib.rs | Registers new commands in the Tauri invoke handler and export list. |
| src-tauri/src/commands.rs | Adds cave_familiar_contract / cave_familiar_analytics Tauri commands. |
| src-tauri/gen/schemas/desktop-schema.json | Updates generated schema to include allow/deny entries for new commands. |
| src-tauri/capabilities/default.json | Grants default capabilities for new familiar read commands. |
| src-tauri/build.rs | Adds new commands to the native command list used for guard/schema generation. |
| e2e/app.smoke.spec.ts | Extends smoke coverage to assert the new demo route renders and shows Stage 1 notices. |
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| switch (status) { | ||
| case 'not_ready': | ||
| case 'loading': | ||
| return 'Loading…'; |
There was a problem hiding this comment.
Good catch, and left deliberately unchanged here — the fix belongs with the Cave-backed source, not this PR.
not_ready currently carries two meanings: the shell's pre-fetch initial state (all five results start at the NOT_READY constant on line 41), and the adapter's "disposed / no ready client". In this PR the shell is only ever driven by MockFamiliarsSource, which never returns not_ready — so the only reachable case is the initial one, where "Loading…" is accurate.
The misleading case becomes reachable when CaveFamiliarsSource lands and the shell is driven by QueryAdapter. Changing the copy now would be speculative and untestable, so it is tracked on #86 alongside the deferred cave-source.ts, where the disconnected state can be distinguished from the initial one and actually exercised.
| familiarContract: vi.fn().mockResolvedValue({ | ||
| id: 'familiar-1', | ||
| present: { soul: true, identity: true, ward: true, memory: true }, | ||
| report: { specVersion: '1.0', pass: true, properties: [], violations: [], warnings: [] }, | ||
| }), |
There was a problem hiding this comment.
Fixed in 826dacd. Added six tests: parameter forwarding for both methods, omission of an unsupplied window/recentLimit (asserted as an exact options object, since absence is the property under test), cache keying on familiar id and on the analytics query, ttl expiry, staleness when the ready client identity changes, and not_ready when there is no client.
| expect(detail.present).toEqual({ soul: true, identity: true, ward: true, memory: true }); | ||
| }); | ||
|
|
||
| it('reports memory absence as a failing contract property without a violation', async () => { |
There was a problem hiding this comment.
Fixed in 826dacd — renamed to "...with a matching violation", which is what the assertions check.
| * Used by the demo build, tests, and the design board states. It always | ||
| * resolves synchronously and never returns anything but `status: 'ok'`. | ||
| * |
There was a problem hiding this comment.
Fixed in 826dacd. The comment now says it returns status: 'ok' for every id the demo fixtures cover, and that an uncovered id resolves { status: 'error', code: 'not_found' } rather than throwing.
…iptions Addresses the review on #89. The query adapter gained familiarContract and familiarAnalytics without tests of its own; the suite only widened the mock client's shape. Adds six covering how they forward parameters, that an unsupplied window and recentLimit are omitted rather than passed as undefined, that the cache is keyed on familiar id and on the analytics query, that the ttl expires, and that both go stale when the ready client identity changes and not_ready when there is none. The omission test asserts an exact options object rather than objectContaining, since the property under test is absence. Also two descriptions that contradicted their code: a mock-source test named "without a violation" that asserts a Persistent Memory violation is present, and a doc comment claiming the mock never returns anything but status ok when an uncovered id resolves not_found. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KacEJmkX5GhkViPUhx9Mie
…here #89 landed the seam without cave-source.ts or mappers.ts, so the comment on main says one implementation exists and that the Cave-backed one arrives with the SDK bump. This branch is that bump, and it reintroduces both files, so the comment would otherwise describe as absent two modules sitting beside it. Restores the wording #89 replaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KacEJmkX5GhkViPUhx9Mie
Moves each demo surface, its stylesheet, and its mock data behind lazy(), so only the read-only production app ships in the default bundle, and scopes Suspense to the demo surfaces rather than the root so App is not wrapped in a boundary it does not need. Also separates the familiar empty states and clears the remaining Rust release build warnings. Rebased onto ab7dc73. The familiars-reads surface added by #89 joins the same pattern: reads-shell and mock-source are imported together inside the lazy chunk, reads-shell.css moves into the component so the chunk carries its own styles, and the surface gets its own Suspense boundary. All four demo stylesheets now emit as separate chunks -- reads-shell 2.96 kB, minimal-macos 30.40 kB, chat-demo 41.84 kB, familiars-shell 58.11 kB -- leaving index.css at 31.17 kB carrying only the production app. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KacEJmkX5GhkViPUhx9Mie
Splits #86 so the Stage 1 work that does not need a new SDK can land now.
Why split
#86 replaced the vendored SDK tarballs (
cave-client,sdk-core) withoutre-pinning
contract-canary.lock.json, which pins each artifact by size andSHA-256:
sdk-core-0.1.0.tgzdc86c6d4c89a574e8bd5cave-client-0.1.0.tgzde16ce13f2c44544adf8The Contract canary checks out the SDK at the pinned revision and verifies the
vendored artifacts against it, so #86 as it stood would have turned
main'scanary red. Re-pinning honestly needs a
releaseManifest, which needs theauthorized release cut — so the vendor bump waits instead.
What lands here
The native familiar contract and analytics read paths, the data-source seam and
its mock implementation, the capability additions, and the reads shell. Vendored
artifacts and
pnpm-lock.yamlare byte-identical tomain.What waits for the SDK bump
src/familiars/mappers.ts,src/familiars/cave-source.ts, and the familiartransport in
src/lib/sdk/native-boundary.ts. Between them they need sixsymbols the pinned tarball does not export:
CaveFamiliarIdentity,CaveFamiliarWard,CaveExecutionDay,CaveFamiliarAnalyticsTransportOptions,canonicalFamiliarContractData,canonicalFamiliarAnalyticsData.Nothing in the app imports either module yet —
mappers.tsis used only bycave-source.tsand its test, andcave-source.tsonly by its own test — soremoving them costs no wiring. The seam's doc comment records what arrives with
the bump.
Verification
tsc --noEmit— 0 errors against the pinned SDKtest:unit:normal— 34 files, 499 passedtest:unit:heavy— 243 passedRust is byte-identical to the tree validated on #86 (
cargo test111 passed,clippy0 issues,fmtclean).🤖 Generated with Claude Code
https://claude.ai/code/session_01KacEJmkX5GhkViPUhx9Mie