feat: add private publisher follow graph - #2950
Conversation
|
@giodl73-repo is attempting to deploy a commit to the OpenClaw Foundation Team on Vercel. A member of the Team first needs to authorize it. |
26ccc1b to
bbf3254
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
bbf3254 to
9b8f03d
Compare
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs real behavior proof before merge. Reviewed July 22, 2026, 1:33 AM ET / 05:33 UTC. Summary Reproducibility: not applicable. This PR proposes a new publisher-follow capability rather than reporting broken existing behavior. The relevant validation is a real after-fix runtime exercise of the proposed API. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Choose and document one maintainer-owned public publisher-feed/follow-graph contract, rebase the selected implementation onto current main, then provide redacted live Convex or HTTP proof covering follow, idempotent repeat follow, authenticated listing, public list visibility, unfollow, and deletion cleanup. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new publisher-follow capability rather than reporting broken existing behavior. The relevant validation is a real after-fix runtime exercise of the proposed API. Is this the best way to solve the issue? Unclear: the implementation is thoughtful and documented, but a public durable social graph is a product and privacy decision, and the branch overlaps an open publisher-feed implementation that must be reconciled first. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a9775fc39b10. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (7 earlier review cycles)
|
9b8f03d to
185bddc
Compare
Patrick-Erichsen
left a comment
There was a problem hiding this comment.
This adds a private, authenticated publisher-follow graph with idempotent follow/unfollow, notification preferences, bounded listing, and API-token HTTP handlers. The authentication binding and separation from trust/install authority are sound, but the implementation does not yet preserve ClawHub's existing visibility and deletion invariants.
LOC: +1274/-1 (14 files)
Findings
-
[P1] Apply linked-user visibility before accepting or returning personal publishers.
requireActivePublisher, list filtering, andisFollowingPublisheronly callisPublisherActive, which checks the publisher row's own deletion flags. ClawHub's canonical public publisher path also rejects a personal publisher whose linked user is deleted or deactivated, while user bans setusers.deletedAtwithout deactivating the publisher row. As written, an authenticated caller who knows the publisher ID can follow a banned personal publisher and retrieve its handle, display name, kind, and image through the private list even though normal publisher visibility hides it. Extract/reuse the full publisher visibility check in follow, list, andisFollowingPublisher, and add linked-user ban/deactivation tests.convex/publisherFollows.ts:22convex/publishers.ts:287convex/users.ts:1730 -
[P2] Purge private follow edges during account and publisher deletion.
publisherFollowsis classified as permanent preference data, but self-service account cleanup never removes rows owned by the deleted follower, and publisher hard deletion never removes rows targeting the deleted publisher. Those rows become permanently unreachable private social-graph data or dangling publisher references. Add bounded, resumable cleanup through the existing follower/publisher indexes, include counts in deletion audit metadata, and cover both lifecycle paths.convex/schema.ts:2693convex/users.ts:353convex/publishers.ts:2014 -
[P2] Define and test the public HTTP contract, including terminal cursor shape. The three
/api/v1/publisher-followsmethods are absent from OpenAPI anddocs/http-api.md. The HTTP handler also exposes the internal Convex{ continueCursor, isDone }result verbatim, includingcontinueCursor: ""at completion even though the request parser rejects an empty cursor; sibling HTTP APIs exposenextCursor: string | null. Add OpenAPI schemas/operations, contract tests, and docs, and normalize terminal pagination at the HTTP boundary (or explicitly specify and test a different public shape).convex/http.ts:318convex/publisherFollows.ts:199convex/httpApiV1/publisherFollowsV1.ts:41 -
[P3] Regenerate the route sourcemap. The source, JS, and declarations contain
publisherFollows, but the committedroutes.js.mapstill maps the old route object. Running the schema build changes the map to include the new property.packages/schema/dist/routes.js:32 -
[P3] Include ordering fields in the new index names.
by_followerandby_publisherboth includeupdatedAt; the checked-in Convex guidelines require index names to include every indexed field. Rename them toby_follower_and_updatedAtandby_publisher_and_updatedAt(or the repository's equivalent convention) and update callers.convex/schema.ts:2700
Best-fix verdict: too narrow. The follow module's idempotency, auth ownership, and bounded reads are reasonable, but the best implementation shares ClawHub's canonical publisher visibility/lifecycle rules with #2948 instead of creating a second publisher-active definition.
Alternatives considered: marking every personal publisher deleted when its user is banned is too broad for unban/recovery semantics; deleting follows on every reversible suspension is also too broad. Prefer a shared visibility helper for reversible states and bounded edge cleanup for permanent deletion.
Authority check: no caller or callee uses follow state to grant official status, publisher membership, publishing permission, registry review, scan success, source trust, local approval, package eligibility, or install authority.
Validation: focused exact-head tests pass (16 passed); static, unit, packages, types-build, e2e-http, and CodeQL are green. Exact-head CI is still unstable because playwright-local-auth / moderation-star failed in the unrelated malicious-skill ban flow (the same matrix failure is present on #2948), and the external Vercel authorization status is failing.
Stack/base: #2950 and dependency #2948 are sibling branches from base 35cd2e51, not a true stack. A synthetic merge conflicts in convex/http.ts, convex/httpApiV1.ts, and specs/README.md; rebase #2950 after #2948 and rerun exact-head CI.
Reviewed exact head b5519cb2e99eecc6215f4ab756d278cfcc0a9e0a; current main observed at 3134a20492f57423ad52ef8b017788a15b8aebd5.
b5519cb to
5557133
Compare
4034523 to
fb78bce
Compare
fb78bce to
5b36e41
Compare
fc85d7d to
86849e9
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing due to inactivity. |
Summary
all/nonedelivery preference for Inbox projectionsAPI
GET|POST|DELETE /api/v1/publisher-followsProduct boundary
Following is discovery state only. It does not grant official status, registry review, source trust, scan success, local approval, or install authority. The preference controls delivery projections; the follow remains available to a future chronological Following feed.
Stack
Depends on #2948. Followed publisher activity is added by #2958. The branch is commit-stacked because cross-fork PRs cannot select another fork branch as their base.
Validation
git diff --check