Repository navigation
Conversation
b646e68 to
d0b87a0
Compare
There was a problem hiding this comment.
🟡 Changes recommended
README event-schema and timestamp-format documentation need clarification before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds principal SSE firehose events with DAG-JSON payloads, API types, dispatch logic, documentation, and tests.
Changes:
- Adds principal event models and generated codecs.
- Emits principal events alongside revocations.
- Adds mixed-stream coverage and README documentation.
File summaries
| File | Summary |
|---|---|
README.md |
Documents the new event kind and payload. |
pkg/fx/app.go |
Encodes and emits principal events. |
pkg/fx/app_test.go |
Tests mixed and live firehose streams. |
pkg/api/types.go |
Adds principal event models. |
pkg/api/types_test.go |
Tests serialization and event helpers. |
pkg/api/json_gen.go |
Adds generated DAG-JSON support. |
pkg/api/gen/main.go |
Includes the new type in generation. |
Review details
Files not reviewed (1)
- pkg/api/json_gen.go: Generated file
Suppressed comments (1)
README.md:119
- The preceding paragraph says each event has the revocation fields (
revoke,path, and so on), but these newly documentedprincipalframes intentionally omit them. Please qualify the earlier field list as applying only torevocationevents, or clearly distinguish the two schemas here; otherwise consumers following this documentation will expect fields that are absent.
The same stream carries `principal` events, which record that every proof a
- Files reviewed: 6/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| The same stream carries `principal` events, which record that every proof a | ||
| gateway cached for a principal's keys is void. Each has `tenant` (the tenant | ||
| DID), `principal` (the principal identifier, unique within the tenant), `cause` | ||
| (the invalidation invocation CID), and `recorded_at` (unix nanoseconds, as for every DAG-JSON time here). Keys are emitted in lexicographic order. The cursor, the inclusive |
There was a problem hiding this comment.
Fixed in 71146ca: the lookup example now shows the integer recorded_at, and the firehose field list is qualified as the fields of a revocation event, with the principal event's fields described in their own paragraph.
The firehose writes principal invalidation records as a second SSE event kind, `principal`, with tenant, principal, cause and recorded_at in DAG-JSON and the cause CID as the event id. api.FirehosePrincipalRevocation is the generated wire type; api.FirehoseEvent is a hand-written sum of the two firehose record types for consumers reading both kinds. GET /revocation/:cid is unaffected. The README documents the new event under GET /revocations/:from. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
d0b87a0 to
71146ca
Compare
|
Superseded by the per-key marker design in fil-one/RFC#30 (f61f474): no principal invalidation record; markers are revoked through the existing /ucan/revoke, and the client's nonce option is #15. Branch kept. |
Description
The firehose emits principal revocations as a second SSE event kind,
principal, with tenant, principal, cause andrecorded_atin DAG-JSON. Existing SSE readers ignore event names they do not match. Part of the Forge S3 tenant IAM work (RFC).Change log
api.FirehosePrincipalRevocation(generated) andapi.FirehoseEvent🤖 Generated with Claude Code