notes group integration: generic channel-host convention + membership#5897
notes group integration: generic channel-host convention + membership#5897arthyn wants to merge 14 commits into
Conversation
Let %groups auto-join/leave and track active-channels for backing agents other than %channels (e.g. %notes), without naming any of them. Convention: a nest whose kind is not %chat/%diary/%heap is backed by the agent literally named by the kind on our ship. %groups pokes it %<kind>-join (carrying [nest group-flag]) / %<kind>-leave (carrying nest), and reads a conventional .../joined/<host>/<name> scry for active-channels. chat/diary/heap keep their existing %channels fast-path untouched. - join-channels / leave-channels: generic else-branch building the %<kind>-join/-leave poke to [our kind]. Uninstalled kinds just nack (logged in go-agent, harmless). - is-joined helper + the two active-channels scry sites (group init, channel-add repair) gain the generic /joined branch. No %groups state change. Compiles against the develop groups desk.
Clean-cut removal of the three notes-channel hacks now that %notes is
group-aware and %groups drives it via the channel-host convention.
- createNotesChannel: create the notebook through the %notes HTTP API
(POST /notes/~/v1/notebooks with {title, group}), reading the
server-assigned flag straight from the response — no poke+poll, no
forced-public visibility. Binds the notebook to the group.
- add Urbit.requestJson + an api.requestJson wrapper (authed JSON request to
an arbitrary ship path; reauth-on-403) for first-class agent HTTP APIs.
- initApi: fold each group's active-channels into joinedChannels so channels
backed by non-%channels agents (notes) reconcile membership normally.
- queries.ts setLeftGroupChannels: drop the notes/% force-join special-case;
notes channels now reconcile like any other channel.
api + shared typecheck clean.
…-channels Correction to the previous client commit. toClientChannel already computes currentUserIsMember kind-agnostically from group reader-roles (open channel or the user holds a reader role) and sets type/contentConfiguration — for notes channels just like chat/diary/heap, on every group sync. So notes membership is already handled the same way as all group channels. The only real issue was that setLeftGroupChannels (the %channels-membership reconcile) flipped group channels not in joinedChannels to not-a-member, clobbering that reader-role membership for notes (which %channels doesn't track). - setLeftGroupChannels: scope the flip to %channels-backed kinds (chat/diary/heap) so notes (and any future channel-host kind) keep the membership toClientChannel derived. Replaces both the old force-join hack and the active-channels approach. - initApi: revert the active-channels -> joinedChannels fold. active-channels remains a %groups-internal signal that drives backend auto-join (Part B); it is not the client's membership source. api + shared typecheck clean.
- %groups: generic group-channel-active poke (new mar/group/channel-active) lets a backing agent (e.g. %notes) report channel join/leave so %groups can keep active-channels current without subscribing to it; resolves the group from the poke and no-ops if absent. is-joined uses %gu for all kinds; the %add repair reuses it. - client: notes channels created via the %notes HTTP API (deterministic flag); membership reconciled from each group's active-channels (setJoinedNotesChannels, fed from initApi) rather than %channels' joined list, which doesn't track notes. setLeftGroupChannels scoped to %channels-backed kinds. api + shared typecheck clean.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c5ce48d93
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- useChannelHooksPreview: skip notes channels (hooks are a %channels feature; /v1/hooks/preview bad-watch-paths on %channels for a notes nest). - leaveGroupChannel: notes channels leave via a %notes poke (unsubscribe + report leave to %groups) instead of poking %channels, which rejects the unknown nest.
There was a problem hiding this comment.
💡 Codex Review
Route notes channel joins through %notes
When a user has left a notes/... group channel, the Join action still calls api.joinChannel(channelId, groupId), which pokes %channels. This patch already special-cases leaving because notes channels are not %channels-backed, so the same notes nest is rejected here and the catch block rolls currentUserIsMember back to false; that leaves users unable to rejoin notes channels they intentionally left.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- joinGroupChannel: notes channels join via a %notes poke (subscribe + report to %groups) instead of poking %channels. - ChatOptionsSheet: hide "Mark as read" for notes channels — they have no %channels/%activity unreads, so it doesn't apply (and would poke %activity with an unknown source). (Channel meta/rename already works for notes via api.updateChannel → %groups; only the writers sub-pokes hit %channels and are left as-is for now.)
Note that the channel-host active-channels report comes from third-party agents like %notes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c9ed8f7a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0d45502a4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
arthyn
requested review from
Fang-,
dnbrwstr,
latter-bolden,
mikolajpp and
patosullivan
The %group-channel-active poke arm was added without declaring the mark, so the discipline wrapper rejected the poke before the arm ran (active-channels stayed stale and notes membership never reconciled for other clients). - import the mark (/% m-group-channel-active) and add it to the discipline's poke-mark allow-list. - add a named $channel-active type to the groups sur and use it as the mark's sample — discipline marks need a single named type (an inline tuple fails the /% build / vale derivation).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49f500a6b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const res = await api.requestJson<NotesCreateResponse>( | ||
| '/notes/~/v1/notebooks', | ||
| 'POST', | ||
| { title, group: { host: groupHost, flagName: groupName } } |
There was a problem hiding this comment.
When a notes channel is created with a description, this request only sends title and group to %notes; the description is only inserted optimistically into the local DB. Since the backend-created group channel has no description to return on the next init/sync, the user's description is lost after refresh, unlike the previous addChannelListingToGroup path which sent it in meta.description.
Useful? React with 👍 / 👎.
createNotesChannel dropped the `readers` it received, so the POST body
only carried {title, group} and the %notes host registered the group
channel with no reader roles — every notes channel created open,
defeating the group's can-read gate for private channels.
Thread `readers` from createChannel through createNotesChannel into the
POST body; the %notes host registers the group channel with the correct
reader roles.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Joining/leaving a channel-host channel (e.g. %notes) mutated the group's
active-channels set but never notified subscribers, so a second same-ship
client only learned the change on its next full init sync.
Add a local-only %active-channels r-group response variant carrying the
per-nest membership delta. The %group-channel-active poke arm now emits it
on /v1/groups (a from-self-only path, so it never crosses the wire) after
updating active-channels. The client reduces it to the existing
join/leave GroupUpdate, reconciling membership without a re-sync.
- sur/groups + groups-ver (v9): new [%active-channels =nest joined=?] arm
- groups-conv: drop it from v7/v2 backcompat diff (compile guard)
- groups-json: encode { active-channels: { nest, joined } }
- groups.hoon: relax discipline on group-response-1 (its carried type
changed; must not pin into version negotiation) + emit the local fact
- client: parse the variant into joinChannel/leaveChannel
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Factor the active-channels mutation + local %active-channels emit into a shared set-active-channel helper and route the take-channels create/join/ leave paths through it too, not just the %notes %group-channel-active arm. active-channels is now the single authoritative membership signal for every channel kind — a second same-ship client reconciles join/leave for built-in chat/diary/heap channels the same way it does for notes, without relying on a separate cross-reference to %channels. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6846255fed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return joined | ||
| ? { type: 'joinChannel', channelId: nest, groupId } | ||
| : { type: 'leaveChannel', channelId: nest }; |
There was a problem hiding this comment.
When a %active-channels join arrives for a notes channel that is not already in a local nav section (for example a same-ship client just created or joined it), this is converted into the existing joinChannel update. handleGroupUpdate processes that via db.addJoinedGroupChannel, which does more than flip membership: if no section row is present it inserts a group_nav_section_channels row under the bare groupNavSectionId 'default' even though group section IDs are stored as ${groupId}-${sectionId}. That can fail the FK or leave an invalid nav-section entry for a membership-only delta; consider a separate update path that only sets currentUserIsMember.
Useful? React with 👍 / 👎.
A %channel %edit r-group update is reduced into a db.Channel via
toClientChannel, which has no role context in the reducer and so computes
currentUserIsMember from readers ∩ {} — false for any role-restricted
channel. The updateChannel apply step then wrote that false, making a
channel an admin had just restricted appear "left" until a full refresh
(insertChannels excludes currentUserIsMember on upsert, so the forced
syncGroup couldn't repair it).
currentUserIsMember is the joined flag (addJoinedGroupChannel /
removeJoinedGroupChannel set it) and is owned by the full group sync
(which has the user's roles) and the active-channels join/leave path. A
channel edit must not redefine it, so strip it from the updateChannel
apply. Previously latent — only open channels (readers=[] → member=true)
were reachable; role-restricted notes/group channels now expose it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2da4a0242
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| :: scry with a loob. | ||
| =/ =path | ||
| /(scot %p our.bowl)/[p.nest]/(scot %da now.bowl)/joined/(scot %p p.q.nest)/[q.q.nest] | ||
| .^(? %gu path) |
There was a problem hiding this comment.
For any non-built-in channel kind whose backing agent is missing or does not implement /joined/<host>/<name>, this naked %gu scry can crash the groups agent during init/channel creation paths that call is-joined. The previous code skipped non-%channels kinds entirely, and other scries in this desk first check the target agent/path (for example with /$) before reading a value; add the same existence/fallback handling here so unsupported custom channel kinds read as not joined instead of aborting the update.
Useful? React with 👍 / 👎.
Fang-
left a comment
Fang-
left a comment
There was a problem hiding this comment.
As discussed out of band, I don't hate this. Some comments/questions below. In particular, not clear to me why we would want to do dynamic marks for the status update pokes that come out of groups agent.
Per review (Fang): the per-kind dynamic marks for the channel-host
join/leave pokes aren't needed — the nest kind already routes the poke to
the backing agent, and the payload shape/meaning are fixed. Replace the
(crip "{kind}-join") construction with static %group-channel-join /
%group-channel-leave.
Make the convention's marks first-class on the groups desk, symmetric with
%group-channel-active: add channel-join/channel-leave types to sur/groups,
new mar/group/channel-{join,leave}.hoon, and register both in the agent
discipline. Pokes are cast to the canonical types.
Also tidies the set-active-channel / channel-active doc comments.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Summary
This improves %groups ability to have non-%channels agents integrate with it as well as makes the integration with our client more "real" and less of a hack. This is in service of us trying to build an actual UI integration with %notes.
Companion to arthyn/notes#7
Changes
Adds support for
%notesnotebooks as a group channel type, via a generic "channel-host" convention so%groupscan drive a backing agent it doesn't know about.%groupsagent%chat/%diary/%heapis backed by the agent named by the kind.join-channels/leave-channelspoke%<kind>-join/%<kind>-leave;is-joinedreads a conventional/joined/<host>/<name>scry foractive-channels. chat/diary/heap keep their existing%channelsfast-path untouched. No%groupsstate change.group-channel-activepoke (+mar/group/channel-active) lets a backing agent report channel join/leave soactive-channelsstays current without%groupssubscribing to it.Client
%notesHTTP API (POST /notes/~/v1/notebooks), which returns the server-assigned flag deterministically (addedUrbit.requestJson+ anapi.requestJsonwrapper).active-channelsset (setJoinedNotesChannels, fed frominitApi) rather than%channels' joined list, which doesn't track notes.setLeftGroupChannelsis scoped to%channels-backed kinds.%notesinstead of%channels;%channels-only affordances that don't apply to notes are suppressed (hooks preview, mark-as-read).How did I test?
%groupscompiles against thedevelopgroups desk; the%notesdesk's unit suite passes (84/84), including the state migration.active-channelspopulation (host- and subscriber-side).Risks and impact
%groupsagent (channel-host convention)Additive: the generic channel-host path only fires for non-
%channelskinds (unknown/uninstalled kinds no-op), and notes-specific client branches are gated on thenotes/id prefix, so chat/diary/heap behavior is unchanged. No%groupsstate migration.Rollback plan
Revert the commits on this branch. There is no
%groupsstate migration to unwind, and the client changes are gated to notes channels, so reverting restores prior behavior cleanly. The%notescompanion desk is deployed separately and is inert for clients without these changes.Screenshots / videos