feat(native): familiar contract and analytics reads - #84
Closed
BunsDev wants to merge 1 commit into
Closed
Conversation
Add the two native commands for the familiar detail reads Cave promoted into Client v1 (OpenCoven/coven-cave#5288) and the SDK now serves (OpenCoven/sdk#104): cave_get_familiar_contract GET /api/client/v1/familiars/:id/contract cave_get_familiar_analytics GET /api/client/v1/familiars/:id/analytics Both go through the same bounded path every canonical read uses: one CaveReadPath variant with validated parameters, one command taking `handle` and `operation`, one capability permission, and one entry in each reviewed command table. A familiar id is held to the slug allow-list Cave itself enforces, which is narrower than a conversation id: `.` and `~` are legal there and not here, because Cave's own allow-list does not carry them. An id this host would have to percent-encode is one Cave could never name, so it is refused before the wire rather than sent to be refused. The analytics narrowing is bounded the same way. Cave refuses an unknown window or an out-of-range recent count rather than correcting it, so the same values are refused here, and the query carries only what the caller asked for -- an implied default would become a refusal at the other end. The webview binding for these commands lands with the vendored SDK bump; the host is complete and exercised by its own tests in the meantime. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KacEJmkX5GhkViPUhx9Mie
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The new commands follow the existing validated native boundary pattern and the supporting guard/capability/schema/test updates appear complete and internally consistent.
Pull request overview
This PR adds Stage 1 “read” support in the Tauri native host for the Familiars surface integration, exposing two new Cave client-v1 read commands (contract + analytics) behind the same constrained/validated native boundary pattern used by existing canonical reads.
Changes:
- Added
CaveReadPathvariants + validation + route/query construction for familiar contract and analytics reads in the native transport. - Added two new Tauri commands (
cave_get_familiar_contract,cave_get_familiar_analytics) and registered them in the reviewed command tables. - Extended capabilities / generated schema / specification guard tests to include the new commands and permissions.
File summaries
| File | Description |
|---|---|
| src/specification-guards.test.ts | Extends the guard expectations to include the two new commands and allow-permissions. |
| src-tauri/src/transport.rs | Adds validated read-path variants plus query construction for familiar analytics narrowing and routes for both endpoints, with unit tests. |
| src-tauri/src/lib.rs | Exports and registers the two new commands in the Tauri invoke handler and smoke test command list. |
| src-tauri/src/commands.rs | Implements the two new Tauri commands and adds them to the registered command list. |
| src-tauri/gen/schemas/desktop-schema.json | Adds allow/deny permission const entries for the two commands to the generated schema. |
| src-tauri/capabilities/default.json | Adds the two allow-permissions to the default capability set. |
| src-tauri/build.rs | Extends the reviewed native command table for permission/schema generation alignment. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 3, 2026
Member
Author
|
Superseded by #86, which includes this same Stage 1 Task 3 work (native 🤖 Generated with Claude Code |
BunsDev
added a commit
that referenced
this pull request
Sep 4, 2026
…ine speed
The windows-supervisor-behavior job is red on main. It is not red because of
anything this suite exists to catch.
Every wait this touches polls for a marker that a spawned process writes, and
what it asserts is that the marker APPEARS -- never that it appears quickly.
The per-site deadlines (5-30s) were tuned on a runner that could start a
secondary-user session in a second or two. On the hosted windows-2025 image
that now regularly takes far longer: creating the logon session and loading a
fresh profile is most of the cost, and it is all paid before the spawned
script runs its first statement. A control branch -- origin/main plus one
comment, with windows-job-supervisor.{cs,test.ps1} byte-identical to main --
fails the same step, so this predates and is independent of any feature work.
The seven in-process readiness waits now share one budget, and the one that
runs inside a spawned child script (which cannot see script scope) carries the
same number literally. This bounds patience, not behaviour: a marker that
never arrives still fails, and the job's own 20-minute timeout still bounds
the run. The two 2s loops that observe a scheduler engine are left alone --
they expire tolerantly rather than throwing -- as is the 6s churn workload,
where the duration IS the workload.
Also name the cause when something throws. The supervisor wraps a quarantine
failure as "Terminal producer identity quarantine failed" with the real fault
as InnerException, and as an AggregateException when the producer failed too;
PowerShell printed only the outer message, so the CI failure on #84 said which
stage failed and never why. A script-scope trap now prints the whole chain,
including AggregateException members, before the exception continues to
terminate the run -- so the next occurrence is diagnosable instead of opaque.
Verified with pwsh 7.6.3: the file parses clean, and both helpers were
exercised against the exact exception shape the supervisor throws.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KacEJmkX5GhkViPUhx9Mie
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
Stage 1, Task 3 (steps 1–2) of the Familiars surface integration — plan of record:
docs/superpowers/plans/2026-09-02-familiars-integration.md.Adds the two native commands for the familiar detail reads Cave promoted into Client v1 (
OpenCoven/coven-cave#5288) and the SDK now serves (OpenCoven/sdk#104,#105):cave_get_familiar_contractGET /api/client/v1/familiars/:id/contractcave_get_familiar_analyticsGET /api/client/v1/familiars/:id/analyticsEach follows the same bounded path as every canonical read: one
CaveReadPathvariant with validated parameters, one command takinghandleandoperation, one capability permission, and one entry in each reviewed command table (build.rs,commands.rs,lib.rs,capabilities/default.json, the generated desktop schema, andspecification-guards.test.ts).Bounds
Familiar ids are held to the slug allow-list Cave itself enforces (
[a-z0-9][a-z0-9_-]{0,63}), which is deliberately narrower than a conversation id:.and~are legal there and not here, because Cave's allow-list does not carry them. An id this host would have to percent-encode is one Cave could never name, so it is refused before the wire rather than sent to be refused.Analytics narrowing is bounded the same way. Cave refuses an unknown
windowor an out-of-rangerecentrather than correcting it, so the same values are refused here; and the query carries only what the caller set, because an implied default would become a refusal at the other end.Scope
The webview binding for these commands is not in this PR. It imports
canonicalFamiliarContractData/canonicalFamiliarAnalyticsDatafrom the SDK, which requires bumping the vendored tarballs — and that is blocked on an SDK release-candidate cut (see below). The host is complete and exercised by its own tests in the meantime; nothing in the webview calls these commands yet.Validation
cargo test— 112 passed, including 3 new: the familiar-id allow-list (accepted and refused shapes, incl...,a/b,dot.name,-leading, 65 chars), the analytics narrowing bounds, and route/query construction.cargo clippy --all-targets --all-features -- -D warningsclean;cargo fmt --checkclean.specification-guards50/50 — the three command/permission/schema guards failed first, then passed once implemented.pnpm typecheckclean; unit suite 473 passed / 61 skipped across 31 files.Follow-up (blocked, needs a decision)
Task 3 step 3 vendors the new SDK tarballs and re-pins
contract-canary.lock.json/phase1-conformance.lock.json. That is blocked: sinceOpenCoven/sdk#74,create-release-artifacts.mjsis publication-only and refuses whilepublishingEnabled: false. The canary passes today only because the pinnedacc38488predates that change, so re-pinning to any newer SDK revision requires an authorized release-candidate cut (RELEASING.md§1–4).