Skip to content

feat(relay): gate artifact writers on NIP-AR support - #2735

Open
tellaho wants to merge 2 commits into
mainfrom
tho/page-record-signing
Open

tellaho wants to merge 2 commits into
mainfrom
tho/page-record-signing

Conversation

@tellaho

@tellaho tellaho commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Overview

Category: improvement

User Impact: Plugins receive editable-record support only in communities that advertise the required relay protocol.

Problem: The app currently offers its artifact writer even when a relay does not advertise NIP-AR, whose revision and conflict rules editable records require.

Solution: Expose the existing kind 45010 artifact capability only when NIP-11 supported_extensions contains nip-ar. Preserve the existing signing, retry and refusal behavior.

This gates capability discovery. The pre-existing raw native and agent signing APIs still admit kind 45010; this change does not add protocol discovery inside those endpoints. No plugin UI, record model, documentation or tool descriptions change.

Changes

File changes

browser-host/relay-broker.mjs
Read NIP-AR advertisement and include kind 45010 in the broker's advertised writer kinds only when supported.

src/features/relay/native.ts
Expose the native transport's artifact capability only on relays advertising NIP-AR.

browser-host/relay-authority.test.mjs
Exercise real NIP-11 parsing through the broker and transport for supported, absent and malformed advertisements, including artifact signing on the supported path.

src/features/relay/native.test.ts
Cover capability absence and supported signing while retaining identical-byte retries and typed refusal checks.

crates/agent-controller/src/app_agent/tests.rs
Verify kind 45010 preserves the saved owner attestation and a deletion envelope without chat timestamp metadata.

browser-host/relay-broker-live.test.mjs
Advertise artifact support in the existing live-socket fixture that exercises artifact publication outcomes.

browser-host/workflow-broker.test.mjs
Update expected writer kinds for fixtures without NIP-AR support.

Reproduction steps

  1. Run the broker authority and native transport tests. Missing, empty, unrelated and malformed extension advertisements must omit configArtifacts.
  2. Advertise supported_extensions: ["nip-ar"]; the transport must expose the kind 45010 artifact signer and publisher.
  3. Run the agent-controller tests; artifact deletion signatures must retain the owner proof and exclude chat timestamp tags.

Validation

  • The ten negative advertisement cases fail against the unchanged base production files, confirming that they catch unconditional capability exposure.
  • Mandatory commit hooks passed at a69cb66d329203cd2e9b4f883226fc7ef8221b64.
  • BUZZ_TEST_WORKERS=2 git push: TypeScript and related unit tests, design types/guards, workspace Clippy with all targets, and machine push-policy checks passed at the same head.
  • No browser journeys added or removed: protocol discovery and signing are covered at the transport and host layers.
  • Independent review, hosted CI (including Rust test execution) and native/live-relay/human acceptance remain pending. This PR remains draft.

Expose the existing kind 45010 artifact capability only when NIP-11 supported_extensions advertises nip-ar. Preserve native retries and refusal handling and cover broker discovery and agent deletion envelopes.

Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho

tellaho commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Review round 1 — Princess Donut (AI review agent, posted from Taylor's account on Taylor's behalf), head a69cb66d329203cd2e9b4f883226fc7ef8221b64

Verdict: no blockers. 9/10. The gate is small, symmetric where it matters, and the regression cases genuinely bite against the unchanged base. Hosted CI is fully green at this head, including Rust and tool integration and every browser journey shard.

What I checked

  • The browser broker reads the NIP-11 supported_extensions list once per relay. It adds kind 45010 to the session's writer kinds only when nip-ar is present. The browser transport already derives its artifact writer from those kinds, so it disappears when support is absent. The new end-to-end test uses real NIP-11 parsing, the broker session and the browser transport. It covers missing, null, empty, unrelated and string-instead-of-array advertisements, and confirms that no extra discovery request is made.
  • The native transport reuses the discovery document it already fetches; nothing new is read, timed or polled. Its artifact writer keeps the same kind check, viewer check, identical-byte retry and typed refusals as before.
  • No mainline community regresses. The production relay advertises nip-ar unconditionally, beside its artifacts descriptor. Agent config already required nip-ar through its own relay-support check. The bundled Solo pull flow loses its writer only on relays that never advertised NIP-AR.
  • The agent-controller additions are tests only. They pin existing behavior: kind 45010 keeps the saved owner attestation, rejects a forged one and carries no chat ms tag.

Optional findings

  1. Low — writer-kind asymmetry between hosts. Without NIP-AR, the broker now drops 45010 from the general writer's advertised kinds, but the native transport still lists it among its writer kinds. Neither path changes what the raw sign or publish endpoints admit, and no current consumer queues 45010 through the general writer, so nothing breaks. However, the description's statement that raw admission is unchanged reads as if nothing about the broker's general writer changed. A sentence noting that the broker's advertised general writer kinds now follow the gate would make that clear.
  2. Low — the gate checks only the token, not the descriptor. The existing agent-config relay-support check requires nip-ar plus an artifacts descriptor with version 1 and revision kind 45010. This gate accepts the token alone. A relay that advertised nip-ar with a different artifact descriptor would therefore get a writer but no config support. No relay does that today, so this is optional. Sharing one predicate would stop the two definitions from drifting.
  3. Low — the deletion test's owner-proof assertion is loose. The new deletion-envelope test accepts any extra tag named auth, so it would still pass if the signer attached a different attestation. The expanded bounded-kinds test already proves that 45010 carries the saved attestation, so coverage exists. Asserting the exact saved auth value here too would make the test's name accurate.

Nothing else rose above nit level. Merge readiness still requires the human/native acceptance named in the description and a required approving review; this comment is not an approval.

@tellaho
tellaho marked this pull request as ready for review November 7, 2026 03:06
@tellaho
tellaho requested a review from a team as a code owner November 7, 2026 03:06

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 No production correctness or security defects identified. One non-blocking test-maintenance finding below. Native runtime and real-relay persistence remain unverified.

Keep empty-content and signature assertions in the existing deletion-envelope test, which verifies exact owner proof and forged timestamp removal.

Co-authored-by: Carl <acda9e433d19dcd0e6b6840f7f4b98f3a56f1fab98049d444c087019e6d36560@feedback.communities.buzz.xyz>

Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho

tellaho commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Addressed the duplicate artifact-deletion test in 8204b9d975af834a172e3144efa511829a85d194.

Removed the overlapping fixture and moved its empty-content and signature-verification assertions into signs_artifact_revisions_without_a_timeline_ms_tag. The retained case also checks the exact saved owner attestation and removes a supplied forged ms tag. Production behavior is unchanged.

Validation at that commit: cargo test -p buzz-agent-controller passed all 99 tests (93 unit, 6 integration), with no failures. Mandatory commit hooks and BUZZ_TEST_WORKERS=2 git push passed, including TypeScript/related tests, design checks and workspace Clippy. The initial push hit timeouts in unchanged Vite-config/entity-navigation tests; the reduced-concurrency run passed without changing tests or timeouts. Native runtime and real-relay persistence were not exercised by this test-only cleanup; hosted CI for the updated head is pending.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants