docs(protocol): propose simplified channel artifacts - #7791
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🔐 Codex Security Review
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a8703e992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Channel read permission governs every artifact read, including lookups, history, search, previews, counts, and live updates. Threads inherit their channel's audience. Membership and visibility changes take effect on subsequent reads and deliveries. | ||
|
|
||
| Write permission means permission to post a kind-9 message in the channel, including authentication, token restrictions, moderation, and archive checks. This includes agents and, where channel policy permits, nonmembers of open channels. Artifact writes use `channels:write`. |
There was a problem hiding this comment.
Align the artifact scope with channel write permission
Use messages:write here, or explicitly require both scopes rather than defining artifact authorization as equivalent to posting a kind-9 message. In this repository, kind-9 writes require MessagesWrite (crates/buzz-relay/src/handlers/ingest.rs) while ChannelsWrite is documented for creating and updating channels (crates/buzz-auth/src/scope.rs); consequently, a least-privilege agent or guest token that can post messages would satisfy every stated permission in this paragraph and the table below but still be unable to create or edit an artifact. This also works against the product contract's requirement to validate the design against the intended human/agent access model.
AGENTS.md reference: AGENTS.md:L13-L18
Useful? React with 👍 / 👎.
|
|
||
| Defines `kind:45010` for editable records called **artifacts**. Each artifact has one home channel and may be attached to a thread. Its home determines who can read it. Any number of artifacts, including of the same type, may share a channel or thread. | ||
|
|
||
| The relay manages identity, access, and revisions. Clients define the content types, such as `buzz.task` or `buzz.project`. |
There was a problem hiding this comment.
Avoid introducing a second
buzz.project identity
Do not advertise buzz.project as an artifact type without distinguishing it from the repository's existing Project entity. VISION_PROJECTS.md and NIP-MP already define projects as kind:30621 coordinates, whereas this example gives a project an artifact UUID and channel-scoped revision chain; clients following the example would therefore create two incompatible objects called a Buzz project, and task references could not resolve against the existing project routes or repository membership. Use a different type name or define an explicit mapping to the canonical 30621:<pubkey>:<d> project coordinate.
AGENTS.md reference: AGENTS.md:L13-L18
Useful? React with 👍 / 👎.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear for the proposal-only scope
Reviewed exact head 9a8703e992ada58194bd98bc2ad14c4c76481f48 against base 77729abfb692b25a0f4ec4a69add86af2e32c0dd. The complete delta is the new 102-line docs/nips/NIP-AR.md; no implementation changes. No blocking finding. This is not an implementation-readiness certification or an approval review.
The draft fits the relay-owned, channel-scoped product direction: stable community-local identity, atomic prev-based revisions, access checks across read surfaces, source-and-destination write authorization for moves, durable source removal, and terminal deletion. I traced create/edit/conflict, anchor loss, rights changes, moves/history, deletion, current-state filtering, and client fallbacks. The explicitly deferred query/pagination/error/limit-discovery and removal/replay wire formats, numeric limits, database layout, and scale guarantees remain deferred.
Nonblocking clarifications
- Unchanged anchors: line 61 explicitly permits edits after anchor loss. Read that as grandfathering an unchanged
root, including the delete snapshot that preserves it. To avoid “supplied” being read as every complete snapshot, consider “On creation, or when changed from the previous accepted revision...” for the liveness check. The stated exception makes this clarification, not a blocker. - Token scope composition: line 67 selects
channels:writewhile borrowing kind-9 posting eligibility, including token restrictions. ExistingMessagesWriteandChannelsWriteare distinct scopes. Before implementation, say whetherchannels:writereplaces or supplements the message scope, with other posting checks unchanged. I do not treat the explicit artifact-specific scope as a deployed authorization regression.
Deferred work and existing suggestions
Current-state queries already require current tags only and omission of deleted artifacts. A future live filtered projection must handle leaving a filter and reconnect reconciliation, but this draft does not define that subscription surface. Requiring its full delivery design now would expand the agreed scope.
The illustrative buzz.project name and relationship tag do not define a replacement for NIP-MP kind:30621 or grant repository authority. The existing bot suggestion is useful future schema/naming feedback, not a blocker on this envelope proposal. GitHub history contains no human ruling adopting either bot suggestion; the author's closure of #7771 alone is not evidence that maintainers rejected that design.
Validation limits
Source-only review and fresh independent adjudication on the pinned Blox host; no checkout, builds, tests, or PR-code execution. One existing exact-head CI snapshot showed 8 successful and 28 skipped checks, with no pending or failing check. Application suites were path-skipped, and the substantive Codex security-review job was skipped with its required-range comment still outstanding. Those checks do not establish runtime correctness or security-review completion. No CI reruns or monitoring.
|
We've been running a Linear-style tracker for humans and agents on a Buzz fork since August (about 600 issues, plus decisions, saved views and automation settings). Our records already have NIP-AR's shape: a stable id, full snapshots, and a predecessor pointer, so we'd like to converge on this envelope. A few requirements from running it, smallest first:
Happy to contribute relay tests or an implementation once the envelope settles, and to share what we learned on the task UX (capture from a message, agent execution with independent review). |
Thanks!! We're taking 1, 3, 5 and 6: explicit idempotent retries that say whether the conflict head is readable, a non-disclosure rule for create collisions, delete limited to channel owners/admins as a soft delete, and messages:write. For 4, we're adding the access and retention rules for history now; the paginated history API and the public acceptance position (2) go in the implementation-ready sync follow-up. When we start landing this would love to have you test it out! |
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: one protocol recovery contradiction
Reviewed 5438f6679ae86b274aa0cb39d1433872a0ef1bc3 against base 77729abfb692b25a0f4ec4a69add86af2e32c0dd. The entire change is a 109-line proposal-only NIP; this is not an implementation-readiness review.
The earlier scope/anchor clarifications are resolved. The newly added moderation contract conflicts with the existing delete envelope: see the inline P2. Merge criterion: specify a valid, non-disclosing delete after the current revision has been redacted. This needs a document-level rule, not an implementation or a new wire protocol in this PR.
Reviewed identity/CAS retries, channel authority, moves and history isolation, delete/restore/redaction/retention, current-state queries, and client fallbacks, with independent authority review. Existing writer-authorized moves and illustrative project names are not additional blockers. Explicitly deferred sync/query wire details remain deferred.
Validation: pinned-source review and clean git diff --check on Blox; no checkout, builds, tests, or PR-code execution. Existing CI was not all green (PostgreSQL failures and an in-progress desktop smoke lane at inspection); no reruns or monitoring, and no claim that those results validate this protocol proposal.
|
|
||
| ## Moderation and retention | ||
|
|
||
| Relays MUST reject kind-5 deletion requests targeting artifact revisions with an explicit reason. A NIP-29 kind-9005 removal targeting a revision redacts it instead of deleting it: the relay withholds that revision's title, content, and client-defined tags from every surface, including history, search, and live delivery, and serves a relay-authenticated removal marker that keeps its `d`, `h`, `type`, `op`, and `prev`. Redaction never changes which revision is current, so a redacted current revision can still be edited, deleted, or restored with a new revision. Revision history is subject to the community's retention policy; expiring earlier revisions MUST NOT remove the current revision or break `prev` checks. |
There was a problem hiding this comment.
[P2] Define a deletable envelope for a redacted current revision
This promises that a redacted current revision can still be deleted, but line 89 requires the client’s signed op=delete snapshot to preserve the current title, while this paragraph withholds that title from every read surface. Reproduction at the contract level: create an artifact with title T, redact its current revision with kind 9005, then open it as an authorized channel admin on a fresh client and delete it. The admin receives the removal marker, not T, so cannot construct a valid delete. Omitting/replacing T violates the envelope/preservation rules; a client that cached and copies T instead publishes it in a new deletion revision, which line 89 makes readable by d.
Please define the redacted-current deletion exception (for example, an allowed safe tombstone title and suppression of redacted fields), or explicitly require a safe replacement update first and narrow the promise here. The draft should give the authorized client a valid recovery transition without needing to recover or republish the moderated title. This is separate from the deferred removal/replay wire format.
## Summary Fix the upload/edit smoke-test race observed on main at `6410e685a80d42db0645fadc9bbe710559ef911e`. - Hold the mock upload at the Tauri IPC boundary and release it explicitly after checking edit rejection, rather than assuming menu interaction finishes within one second. - Wait for the Radix menu to unmount: edit dispatch happens in `onCloseAutoFocus`, after the click. Without that wait, the negative assertion can pass before the edit callback actually runs. - Keep the existing attachment-preservation and subsequent successful-edit assertions. No production code or CI coverage is changed. ## Evidence and scope - [Main failure](https://github.com/block/buzz/actions/runs/36189213149/job/108250341388): `opening edit during an immediate photo upload preserves the draft`, including an upload-progress timeout on retry. - Investigated from #7791, which changes only `docs/nips/NIP-AR.md`. Its PostgreSQL failures and video-menu timeout are separate; PostgreSQL passed on this main run, and the video spec passed locally. This PR does not claim to fix those failures or the separate pointer-interception failure seen in the main job. - #7843 already addresses unrelated runtime suites being selected for docs-only PRs. ## Validation - Full desktop `pnpm test`: 6,676 Node tests + 92 jsdom tests passed. - Upload/edit regression repeated 5 times: passed. Holding the upload without waiting for menu teardown failed all 5 runs, demonstrating the second race. - Complete file-attachment and video-attachment Playwright specs: 30 passed. - Pre-push desktop lint, typecheck, full desktop tests, and file-size gate passed at `46e3d4392e5b56dcb74ea864aa19aa10287f27d3`. - `just ci` attempted: initial formatting issue corrected; second run exceeded the 5-minute local limit during Tauri clippy. Not claiming full repository CI green. ## Review / human verification Self-reviewed the diff against the test contract; no production behavior changes. Draft pending human verification and CI. To verify: activate Hermit, then `cd desktop && pnpm test:e2e:smoke file-attachment.spec.ts --grep "opening edit during" --repeat-each=5`. Expect all five runs to pass, retaining the upload in the draft and allowing edit only after upload release. Originating conversation: buzz://message?channel=aa7f2b48-d367-4ca1-be09-116a2eb45b99&id=1b68585ba4483a87f5a453fd7bbdd11d434e332dcb4aff8acfe1f5d2601b6561 Signed-off-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz> Co-authored-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz>
Signed-off-by: Fizz <400e8babadcee6a7f420103f10a2849d84c4a9c71d5bd04f3948c814216648a3@buzz.block.builderlab.xyz> Signed-off-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz>
- Artifacts supersede kind:1621 tasks and kind:30621 projects; update VISION_PROJECTS.md to match - Writes use messages:write - Delete/restore limited to artifact author or channel owner/admin (either participant in DMs); delete is soft, d stays reserved - Add op=restore - Define idempotent retries, conflict head disclosure, and non-disclosing create collisions (deterministic d allowed) - Add moderation (kind-9005 redaction) and retention rules - Clarify root wording, move history access, auth tags, and notification behavior on revisions - Note deferred acceptance order and history pagination Signed-off-by: Honey <8e307ae0076a4dab6b94b036ea3edc7e08f823a625269c1e6919e881a048b4d2@buzz.block.builderlab.xyz> Signed-off-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz>
Keep the vision doc unchanged while tasks and projects on channel artifacts are explored. Signed-off-by: Honey <8e307ae0076a4dab6b94b036ea3edc7e08f823a625269c1e6919e881a048b4d2@buzz.block.builderlab.xyz> Signed-off-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz>
Hold the kind:1621/30621 replacement claim until it is proven in testing. Signed-off-by: Honey <8e307ae0076a4dab6b94b036ea3edc7e08f823a625269c1e6919e881a048b4d2@buzz.block.builderlab.xyz> Signed-off-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz>
5438f66 to
2d3f07d
Compare
Signed-off-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear for the proposal-only scope
Re-reviewed f88e4ab1c97e8ff6484d0d660887eb9b8267744b against base 02c6309f534050e54c8d36f5ce90e48e4dd152b1. The PR remains one new 109-line protocol proposal; the repair changes three lines relative to the previous reviewed proposal.
The prior redacted-title blocker is resolved. The tag table, cardinality rule and deletion rule now consistently forbid a title on deletion. Deleting no longer requires reading or republishing the moderated title. CAS, channel authorization, delete/restore authority and redaction protections are unchanged; restore still requires a new complete snapshot.
No blocking finding in this bounded re-review. One optional marker-metadata clarification is inline. I am not turning the explicitly deferred removal/query/replay wire design into another merge gate or certifying fresh-client wire interoperability. Existing illustrative schemas and accepted product choices remain outside this repair.
Validation: source-only inspection on pinned Blox, with two independent lanes integrated; no checkout, builds, tests or PR-code execution. The existing exact-head CI snapshot had 8 successful and 28 skipped checks, including skipped application suites and substantive security review. This is neither runtime validation nor an approval review.
|
|
||
| ## Moderation and retention | ||
|
|
||
| Relays MUST reject kind-5 deletion requests targeting artifact revisions with an explicit reason. A NIP-29 kind-9005 removal targeting a revision redacts it instead of deleting it: the relay withholds that revision's title, content, and client-defined tags from every surface, including history, search, and live delivery, and serves a relay-authenticated removal marker that keeps its `d`, `h`, `type`, `op`, and `prev`. Redaction never changes which revision is current, so a redacted current revision can still be edited, deleted, or restored with a new revision. Revision history is subject to the community's retention policy; expiring earlier revisions MUST NOT remove the current revision or break `prev` checks. |
There was a problem hiding this comment.
Optional clarification: explicitly retain root when present. Line 89 still requires a delete to preserve the current anchor, but this marker description names d, h, type, op and prev, not root. A fresh client would need that anchor value to delete an attached, redacted current revision without a cached copy. Adding “root when present” to the retained metadata would make that dependency clear.
I do not classify this like the previous title contradiction: root is an envelope field, not one of the fields redaction forbids exposing, and this sentence does not say the retained list is exhaustive. A marker retaining it can satisfy both paragraphs. This is a useful clarification for the explicitly deferred marker wire contract, not a new blocking requirement for this compact proposal.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> * origin/main: fix(relay): fail startup on invalid operator listener config (#7933) fix(db): limit event transactions to listener mention kinds (#7932) feat(relay): deliver pubkey mentions to relay companions (#7793) docs(protocol): propose simplified channel artifacts (#7791) feat(push): support configurable HTTP(S) delivery URLs (#7877) fix(ci): select runtime suites from PR changes only (#7843) test(desktop): synchronize upload edit smoke test (#7903) Signed-off-by: Hayt <211b96e6a2b7f45fd4047988976c7bbbeeda0c15f3ae7b32eec20834b5a55118@buzz.block.builderlab.xyz>
* origin/main: 🤖 docs(nip-fi): remove implementation references from the spec (#7912) fix(relay): fail startup on invalid operator listener config (#7933) fix(db): limit event transactions to listener mention kinds (#7932) feat(relay): deliver pubkey mentions to relay companions (#7793) docs(protocol): propose simplified channel artifacts (#7791) feat(push): support configurable HTTP(S) delivery URLs (#7877) fix(ci): select runtime suites from PR changes only (#7843) test(desktop): synchronize upload edit smoke test (#7903) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
## Summary Implements NIP-AR channel artifacts (spec merged in #7791) on the relay, after a simplicity pass over the earlier two-agent implementation. An artifact is an editable record (kind 45010) with a stable identity (`d`), one home channel (`h`), and full-snapshot revisions chained by `prev`: - **Writes:** one transaction per write. It locks the artifact's head row, requires `prev` to name the current revision, records the revision in a ledger that survives retention, and advances the head. Two competing edits can't both win. A stale edit gets `conflict: artifact head changed`. Resubmitting an accepted event succeeds without applying it twice. - **Permissions:** writes pass the same gates as a kind 9 message in `h`. A move also passes them in the source channel. The move stores the destination snapshot and a relay-signed kind 45011 removal marker for the source in the same transaction, so both channels recover by replay. The marker never names the destination. - **Moderation:** redaction is the ordinary kind 9005 removal. The ledger keeps the ID so `prev` checks still work. Redacting the current revision retires the artifact. - **Reads:** HTTP `/query` and `/count` accept explicit `artifact: current|history` filters with exact multi-character tag matching (e.g. `#assignee`). Generic REQ and search return only current revisions. Kind 5 against artifacts is rejected. NIP-11 advertises the limits. - **Spec:** NIP-AR updated to match: kind 9 permission checks, 9005 redaction and retirement, no creator-only delete rule. A separate small commit routes `insert_channel_head_checked` through the metered writer pool (`acquire_writer`), like every other event write in that module. ### Related issue Follows #7791. No separate issue found. ### Testing - Clippy clean. - Unit lane passes. Failures seen along the way came from agent env leaking into `buzz-acp` tests (they pass in a clean env) and known timing flakes (`keepalive_resets_idle_past_deadline`, two `buzz-agent` databricks tests that passed on retry). This diff doesn't touch those crates. - Postgres lane passes, including new `artifact_postgres_tests` in `buzz-db` and `buzz-relay`. Three presence tests need `REDIS_URL` set. - Docker smoke against a fresh local relay, 29/29: create and idempotent resubmit, update and stale conflict, current, `#assignee` and history queries, generic REQ, kind 5 rejection, delete and restore, move with source marker, private-channel isolation, and 9005 retirement. - A pasteable curl walkthrough of the same flows was shared with the reviewer. Draft until the human hands-on check (AGENTS.md step 3) is confirmed. The pre-push hook was skipped because every lane above had already run on this tree. CI is the gate. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Honey <8e307ae0076a4dab6b94b036ea3edc7e08f823a625269c1e6919e881a048b4d2@buzz.block.builderlab.xyz> Signed-off-by: murderbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Co-authored-by: Honey <8e307ae0076a4dab6b94b036ea3edc7e08f823a625269c1e6919e881a048b4d2@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Smartie <fb3185faacfc3760b6b4fe0085a8878d8dd537f7a7671c7d3184fe1aa1b04df4@buzz.block.builderlab.xyz> Co-authored-by: Fizz <400e8babadcee6a7f420103f10a2849d84c4a9c71d5bd04f3948c814216648a3@buzz.block.builderlab.xyz> Co-authored-by: murderbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz>
Summary
Adds a short, proposal-only NIP-AR: Channel Artifacts in
docs/nips/NIP-AR.md, as a simpler alternative to the closed #7771.prev-based revisions.Remaining design work
This is a compact draft, not a complete implementation-ready wire contract. Concrete query request/pagination/error/limit-discovery formats, removal/replay wire details, and numeric query limits remain follow-up work. Prefer extending shared WebSocket
REQ/ HTTP/querysurfaces rather than adding plugin-specific endpoints. No database layout or scale guarantees are proposed.Related issue
Closest prior proposal: #7771 (closed). Searched existing PRs and issues for channel artifacts; no matching issue found.
Testing
git diff --check.