Projects v3: unify sharing, discussions, and issue ownership - #5792
Projects v3: unify sharing, discussions, and issue ownership#5792thomaspblock wants to merge 2 commits into
Conversation
…oxy (#5799) **Category:** fix (CI) **User Impact:** None — test-only change that unblocks `main` and every open PR. **Problem:** `main` has been red since #5629 landed on `45f4b91a3`: `Desktop Smoke E2E (3)` fails `compact link preview image geometry truncates long titles to one line` on every build (main run 31727837133, and e.g. #5792, #5790). Two independently-green PRs raced: #5629 added the test stubbing its preview image at the raw relay origin (`http://localhost:3000/media/*.png`), while #5627 rewrites sent snapshot media through the authenticated local media proxy (`http://127.0.0.1:54321` in the E2E mock bridge). Merged together, the image request goes to the proxy origin, the stub never matches, and `naturalWidth` stays `0`. **Solution:** Point the route stub at the mock proxy origin, matching the existing `sent link preview media uses the authenticated proxy in compact and rich cards` test in the same spec. **Testing:** Reproduced the failure locally on `45f4b91a3`, then with this fix: targeted test passes, and the full `messaging.spec.ts` smoke suite passes 58/58. Signed-off-by: Thomas Petersen <thomasp@squareup.com> Co-authored-by: Wintermute <3f1797424fd9ad6653a83665c660517777cd7f8c228c0d5907f49e01537f3ca5@buzz.block.builderlab.xyz>
wpfleger96
left a comment
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Combined review consolidated from two independent full passes, pinned to head aebe24ab1b626ad92cc91f526ded2185744eb9ef against merge-base 45f4b91.
The good news first: the #5624 cold-start deep-link blocker is fixed correctly — pending-entity queue with take/ack commands, get_current() coverage on Windows/Linux, launch-callback dedupe, and the pre-mount E2E regression we asked for. The provenance claim in the description also checks out: the first commit's tree is byte-identical to merging #5624's final head into latest main. CI is fully green at this head, and the desktop, Tauri workspace, CLI, SDK, and ACP suites were run clean locally at this exact commit during review.
Requesting changes on four items, detailed inline:
- Assignment state is silently lost once an assignment operation falls outside the bounded comment fetch window (
projectIssues.mjs). - A remote-agent add-repository failure after the project event publishes leaves an unrecoverable dangling project member (
projectOwnerControl.ts/projectRepositoryCreation.ts/ ACP publish path). - The GHCR login retry in
.github/workflows/docker.ymlis unrelated to this PR and should be split out (with a precise read on what it actually does — it's not a broken duplicate). created_atfuture-cap asymmetry between the ACP and desktop owner-announcement paths can wedge an addressable head.
One minor note inline on entity deep-link queue poisoning, and one extraction suggestion: commit aebe24ab's messaging.spec.ts fix addresses a failure that's currently red on every main build — worth landing as a standalone PR so main goes green independent of this PR's iteration cycle.
| * overriding authority while allowing a later observed owner/author decision | ||
| * to be superseded by the affected assignee. | ||
| */ | ||
| function assignmentStateForIssue(issue, issueCommentEvents) { |
There was a problem hiding this comment.
🤖 [IMPORTANT] assignmentStateForIssue() treats the fetched comment set as the complete assignment history, but both desktop fetch paths bound it: hooks.ts caps kind-1 notes at 500 per repo and projectWorkItems.ts at 2,000 shared across every loaded repo. Once newer comments push an older assignment operation out of that window, its assignee silently vanishes from the issue and from “Assigned to me” — and worse, a later self-service operation can omit the real prior head and get reduced with the wrong authority semantics.
Assignment/unassignment operations should be loaded independently and paginated to exhaustion (or snapshotted into a replaceable event) before reducing state. A regression test where the effective assignment predates 500+ unrelated repo comments would lock this in.
There was a problem hiding this comment.
Fixed in 4fd76d0. Added desktop/src/features/projects/assignmentOperationFetch.ts: fetchAssignmentOperationEvents paginates kind-1 notes filtered on #a + #t: [assignment, unassignment] to exhaustion (until-cursor + dedup by id), independent of the bounded comment window. Wired into both fetch paths — fetchProjectIssues (hooks.ts) and fetchProjectsWorkItems (projectWorkItems.ts). A fetch failure now surfaces as a failed section in the load notice instead of silently degrading assignment state. Verified every writer (desktop hook, Tauri managed-owner command, CLI/SDK builders) tags both #a and #t. 6 regression tests.
| if (projectFrame.status === "ok" && projectFrame.events) { | ||
| finish({ events: projectFrame.events }); | ||
| } else { | ||
| finish({ |
There was a problem hiding this comment.
🤖 [IMPORTANT] Partial-success events are discarded here on any non-ok status, but the ACP side publishes the announcements sequentially and can fail after the first event is already live — publish_project_owner_announcements in crates/buzz-acp/src/lib.rs even returns published_events alongside the error, and this handler throws them away.
Concretely for add-repository: if the repository event fails after the project event lands, the project head now references a repository event that doesn't exist, and retrying can't heal it — buildAddedRepositoryEventTemplatesFromHead() sees the coordinate already in the live head and throws (projectRepositoryCreation.ts “already contains” guard) before ever republishing the missing repository. That's an unrecoverable dangling project member.
Either consume the partial-success metadata and resume publication from where it stopped, or make retry detect “coordinate in head but repository head absent” and publish the missing event. Needs a test covering failure of event 2 after event 1 succeeds.
There was a problem hiding this comment.
Fixed in 4fd76d0. buildAddedRepositoryEventTemplatesFromHead now takes repositoryHeadExists and returns resume templates when the live project head already references the coordinate but no kind-30617 repository head exists (the dangling-member state): it publishes only the missing repository event and reuses the live head as the project event, so no pointless created_at advance and the dominated-write guard is skipped (nothing writes the head in resume mode). projectOwnerControl.ts gains PartialAnnouncementPublishError + isDanglingProjectMemberPublish so the owner-agent path resumes in-flight partial failures too. Tests in projectRepositoryCreation.test.mjs and new projectOwnerControl.test.mjs.
| registry: ghcr.io | ||
| username: ${{ github.repository_owner }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
| - name: Retry GHCR login |
There was a problem hiding this comment.
🤖 [IMPORTANT — scope] This GHCR login retry has no connection to Projects v3, isn't mentioned in the PR description, and is an unexplained edit to a credential-bearing workflow inside an 88-file feature PR.
To be precise about what it actually does (so we're not overclaiming): the fork-PR guard holds — a skipped first step has outcome == 'skipped', not 'failure', so the retry can't fire on fork PRs, and the retry has no continue-on-error so a second failure still fails the job. It's a conventional one-retry sequence, not a broken duplicate. But it's applied to only 1 of the 4 GHCR login sites in this file, which undercuts the transient-failure motivation.
Please drop it from this PR and land it separately with rationale, applied consistently to all four login sites if it's needed.
There was a problem hiding this comment.
Dropped in 4fd76d0 — the docker.yml hunk is fully reverted (9 deletions). If the retry is still wanted, I'll propose it as a standalone PR with the correct rationale applied consistently to all login sites.
| .collect::<Result<Vec<_>, _>>()?; | ||
| let mut builder = EventBuilder::new(Kind::Custom(input.kind), input.content).tags(nostr_tags); | ||
| if let Some(created_at) = input.created_at { | ||
| builder = builder.custom_created_at(Timestamp::from(created_at)); |
There was a problem hiding this comment.
🤖 [IMPORTANT] The ACP path (build_project_owner_announcement_events) rejects created_at > now + 300s, but this desktop command accepts any created_at unchecked. These are addressable events (30617/30621) where latest-created_at wins — an accidental far-future timestamp permanently wedges the project/repo head, since nothing can supersede it until that time. Owner-authorized either way, so it's a self-harm footgun rather than a privilege hole, but the asymmetry looks unintentional: other builders in this same file already clamp with created_at.max(Timestamp::now().as_secs()). Mirror the +300s cap (or the clamp) here.
There was a problem hiding this comment.
Fixed in 4fd76d0. The +300s future cap now lives in validate_project_owner_announcement (so both the command path and tests hit it), mirroring the ACP path exactly. Unit test project_owner_announcement_rejects_far_future_timestamps covers the boundary.
| "take_pending_entity_deep_link", | ||
| ); | ||
| if (!pending) break; | ||
| if (!onOpen(pending.href)) return; |
There was a problem hiding this comment.
🤖 [MINOR] If onOpen rejects the head of the queue (e.g. parseEntityLink fails on it), the drain returns without acknowledging — so every later link stays wedged behind the unparseable head forever. The shared golden fixture makes Rust/TS grammar drift unlikely, but ack-and-drop on parse failure would be cheap insurance against a permanently poisoned queue.
There was a problem hiding this comment.
Fixed in 4fd76d0. Unparseable queue heads are now acked and dropped in useEntityDeepLinks.ts, so a bad link can no longer wedge the links behind it. Unmount still leaves valid links queued for the next listener.
Squashed replacement for the projects-design-v3-layout-test branch (supersedes PR #5624, whose head accumulated stuck queued check suites). Projects v3 makes repository work shareable, discussion-aware, and easier to scan in one coherent workspace: - Unified workspace: top-level sections above repository controls in one rounded workspace, with labeled icon headers and consistent creation actions for Issues and Pull Requests. - Repository management: always-available repository selector with an integrated add flow for creating or selecting repositories. - Readable work-item lists: plain-language row context, consistent row density, right-aligned timestamps, deterministic fallback-avatar colors, and Inbox PR metadata that wraps between complete phrases. - Reliable entity links: canonical buzz:// links, preview cards, OS deep-link routing, tab-aware navigation, and entity intents queued until the frontend acknowledges them so cold-start share links navigate reliably. - Related conversations: repository and work-item views surface channels discussing the current entity, with participants, message context, and an explicit 500-result discovery cap notice. - Reversible issue ownership: trusted assignment/unassignment events across Desktop, Tauri, buzz-sdk, and buzz issues, derived chronologically from labeled Nostr notes with owner/self-service authority rules. Also updates webbrowser past RUSTSEC-2026-0257. Co-authored-by: Thomas Petersen <thomasp@squareup.com> Signed-off-by: Thomas Petersen <thomasp@squareup.com>
…ish recovery, timestamp cap Review items from #5792 (wpfleger96): 1. Drop the GHCR login retry from docker.yml — unrelated to Projects v3; will be proposed separately with rationale applied to all four login sites if still wanted. 2. Mirror the ACP path's +300s created_at cap in the desktop publish_project_owner_announcement command (moved into the validate function, with a unit test). These are addressable events where latest created_at wins; a far-future stamp would wedge the head. 3. Ack-and-drop unparseable entity deep links instead of leaving them queued, so one bad link can no longer wedge every later link behind the queue head. Unmount still leaves links queued for the next listener. 4. Make add-repository retry heal a dangling project member instead of throwing. The two-event publish can fail between the project head and the repository event; previously the "already contains" guard made that state unrecoverable. Now: - buildAddedRepositoryEventTemplatesFromHead accepts repositoryHeadExists and returns resume templates (publish only the missing repository event) when the live head references a coordinate with no repository head; - the owner-agent path consumes the ACP frame's partial-success events (PartialAnnouncementPublishError) and resumes in-flight; - the dominated-write guard no longer blocks the heal (resume publishes no project head, so nothing can be clobbered). Covered by tests for the retry-after-event-2-failure flow and the still-a-conflict case when a live repository head exists. 5. Load assignment/unassignment operations to exhaustion instead of trusting the bounded comment windows (500/repo in hooks.ts, 2,000 shared in projectWorkItems.ts). A dedicated #t-filtered query paginates with an id-deduped until cursor and merges into the comment set before reduction; a failed query surfaces as a failed "assignments" section rather than silently dropping assignees. Regression test: an assignment older than 500+ unrelated comments still reduces to an assignee. Co-authored-by: Thomas Petersen <thomasp@squareup.com> Signed-off-by: Thomas Petersen <thomasp@squareup.com>
aebe24a to
4fd76d0
Compare
…ish recovery, timestamp cap Review items from #5792 (wpfleger96): 1. Drop the GHCR login retry from docker.yml — unrelated to Projects v3; will be proposed separately with rationale applied to all four login sites if still wanted. 2. Mirror the ACP path's +300s created_at cap in the desktop publish_project_owner_announcement command (moved into the validate function, with a unit test). These are addressable events where latest created_at wins; a far-future stamp would wedge the head. 3. Ack-and-drop unparseable entity deep links instead of leaving them queued, so one bad link can no longer wedge every later link behind the queue head. Unmount still leaves links queued for the next listener. 4. Make add-repository retry heal a dangling project member instead of throwing. The two-event publish can fail between the project head and the repository event; previously the "already contains" guard made that state unrecoverable. Now: - buildAddedRepositoryEventTemplatesFromHead accepts repositoryHeadExists and returns resume templates (publish only the missing repository event) when the live head references a coordinate with no repository head; - the owner-agent path consumes the ACP frame's partial-success events (PartialAnnouncementPublishError) and resumes in-flight; - the dominated-write guard no longer blocks the heal (resume publishes no project head, so nothing can be clobbered). Covered by tests for the retry-after-event-2-failure flow and the still-a-conflict case when a live repository head exists. 5. Load assignment/unassignment operations to exhaustion instead of trusting the bounded comment windows (500/repo in hooks.ts, 2,000 shared in projectWorkItems.ts). A dedicated #t-filtered query paginates with an id-deduped until cursor and merges into the comment set before reduction; a failed query surfaces as a failed "assignments" section rather than silently dropping assignees. Regression test: an assignment older than 500+ unrelated comments still reduces to an assignee. Co-authored-by: Thomas Petersen <thomasp@squareup.com> Signed-off-by: Thomas Petersen <thomasp@squareup.com>
4fd76d0 to
3198cc7
Compare
2 participants
Summary
Projects v3 makes repository work shareable, discussion-aware, and easier to scan in one coherent workspace. People can copy canonical links, reopen the exact workspace tab, understand issue and pull-request context at a glance, find related channel conversations, and assign or unassign issues across Desktop and CLI.
buzz://links, preview cards, OS deep-link routing, and tab-aware navigation. Reopening the same link re-applies its destination instead of leaving the user on a locally selected tab.buzz-sdk, andbuzz issues. Assignees appear in project views and the assigned inbox, while authorized users can remove assignments directly from the assignee row.Assignment state is derived chronologically from labeled Nostr notes. Issue authors and repository owners may change any assignee; other users may only assign or unassign themselves. Shared golden fixtures keep entity-link grammar and validation aligned across TypeScript and Rust.
The branch also updates
webbrowserto the patched release for RUSTSEC-2026-0257.Related issue
N/A.
Testing
just ci— formatting, lint, typechecking, unit tests, and builds passedcargo test -p buzz-cliand focusedbuzz-sdkassignment tests passedScreenshots
Pull requests explain who opened the request, where it lives, and which branch it comes from; fallback avatars remain visually distinct.
Issues use the same sentence-style hierarchy while keeping status and recency easy to scan.
The wide Inbox detail keeps author, timestamp, and origin context readable beside its metadata rail.
View the complete six-state Projects v3 screenshot set and the compact/wide Inbox comparison.