Skip to content

fix(projects): allow owners to delete agent projects - #6533

Open
wesbillman wants to merge 3 commits into
mainfrom
carl/fix-agent-owned-project-deletion
Open

fix(projects): allow owners to delete agent projects#6533
wesbillman wants to merge 3 commits into
mainfrom
carl/fix-agent-owned-project-deletion

Conversation

@wesbillman

Copy link
Copy Markdown
Collaborator

Summary

  • expose project deletion when the current user directly owns the project or owns/manages its author agent
  • publish a viewer-signed, project-only tombstone after fetching the exact live head, then detect concurrent replacement
  • honor relay-authorized owner-signed tombstones in desktop project read models

Testing

  • pre-push hook: desktop Biome, TypeScript, 5,358 desktop tests, Rust tests, Tauri checks, and file-size ratchet passed at a36f7a4cf
  • cargo check -p buzz-test-client --tests
  • relay E2E coverage added as ignored; not run against a live relay

Co-authored-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman
wesbillman requested a review from a team as a code owner August 22, 2026 00:45

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Requesting changes for one blocking user-visible authorization defect:

P1: Do not expose managed-agent deletion unless the production signer can exercise that authority. canDeleteProject treats every locally managed agent record as sufficient capability via managedAgentPubkeys.has(owner), including legacy/imported records that legitimately have no NIP-OA attestation. But deleteProject always uses signRelayEvent, which signs with the current human identity. The relay accepts that human-signed tombstone only when its database already contains the NIP-OA owner mapping. For a locally managed legacy/imported agent without that mapping, the UI enables Delete and confirmation deterministically fails with must be event author.

Either sign this narrowly constrained tombstone with the managed agent key (the existing project_owner_identity path demonstrates that capability), or stop treating local management alone as deletion authority and expose the action only for relay-verifiable ownership. Please also add coverage through the production signer/relay authorization boundary; the current injected signer/publisher test stubs away the rejection.

The relay authorization, tombstone race handling, and project read-model suppression otherwise look sound at a36f7a4cfca6c66c54a80e4586c130f490031738.

@jedwards27 jedwards27 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.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: f7942167372501576c9f0f589cf2c166882668bb..a36f7a4cfca6c66c54a80e4586c130f490031738
Risk: critical — destructive, identity-bound relay authorization and failure recovery.

Two material issues remain:

  1. P1 — bind the Delete affordance to authority the production signer can exercise. desktop/src/features/projects/projectDeletion.ts:34-39 treats managedAgentPubkeys.has(owner) as deletion authority, although useProjectDeletionAccess.ts:17-25 loads that machine-local list without binding it to the active identity. The operation then signs with the current human identity (projectDeletion.ts:79-84,100-107), while the relay accepts that signer only when its community database says the human owns the agent (crates/buzz-relay/src/handlers/side_effects.rs:249-258). The existing ownership helper explicitly documents that local managed-agent records can diverge from relay ownership (desktop/src/features/profile/lib/identity.ts:134-148). Thus an imported/legacy agent without NIP-OA ownership—or another human identity on the same installation—gets an enabled Delete action in Projects and the sidebar, then deterministically receives must be event author. Remove local-list presence as capability evidence, or perform this constrained deletion with authority that actually owns the author agent. Add a regression spanning the production signer/relay authorization seam; the injected signer/publisher test cannot catch this mismatch.

  2. P2 — reconcile the project cache when publish acknowledgement is uncertain. RelayClient.publishEvent may time out after the relay has durably accepted the tombstone (desktop/src/shared/api/relayClientSession.ts:710-755). deleteProject then exits before its verification fetch (desktop/src/features/projects/projectDeletion.ts:103-113), and useDeleteProjectMutation removes/refetches only in onSuccess (desktop/src/features/projects/hooks.ts:964-975). Both confirmation flows close on failure (desktop/src/features/projects/ui/ProjectCards.tsx:424-428, desktop/src/features/sidebar/ui/SidebarProjectsSection.tsx:238-260). The result is a failure toast plus a still-actionable project from the five-minute cache; retry reports that no live head exists, still without reconciling the durable outcome. Invalidate/refetch the projects query on uncertain failure/settlement and cover accepted publish + lost ACK → deleted project absent. The timeout copy should not assert failure when the outcome is unknown.

The relay's community-scoped coordinate authorization, timestamp-dominating tombstone, project-only target, concurrent replacement detection, and the AlertDialog destructive confirmation looked sound in the reviewed paths. No unrelated schema, identity-storage, or release scope was introduced.

Validation at exact clean head: just desktop-typecheck passed; full just desktop-test passed (5,358/5,358); cargo check -p buzz-test-client --tests passed; git diff --check passed. CI relay/integration, smoke E2E, macOS build, Rust lint, security, cross-compile, and Docker jobs passed when checked. The Unit Tests job failed in sherpa-onnx-sys native static-library discovery after buzz-core passed 307/307; Desktop Core and Windows Rust were still running.

Residual risk: the ignored live-relay E2E and a native destructive UI journey were not run. Existing E2E does not cover the production Desktop signer or lost-ACK reconciliation.

wesbillman and others added 2 commits August 22, 2026 08:30
Co-authored-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
…project-deletion

Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Co-authored-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
@wesbillman

Copy link
Copy Markdown
Collaborator Author

Addressed the requested changes in fe102ad6f and merged current main at 6c40c2f0b:

  • deletion affordance now requires direct ownership or relay-verifiable NIP-OA profile ownership; local managed-agent presence is no longer treated as authority
  • deletion timeout now reports an uncertain outcome and the mutation awaits project-query invalidation on both success and failure, with lost-ACK cache regression coverage

The failed Unit Tests job was unrelated infrastructure (sherpa-onnx-c-api missing during buzz-voice compilation); I reran the failed job after pushing. Pre-push passed at 6c40c2f0b (5,361 desktop tests, Rust tests, Tauri checks, TypeScript, Biome, and file-size gate).

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