pioneer: hosting threads#5872
Conversation
These are redundant with the similarly-named function in ylem's /pkg/runtime/pioneer/src/Pioneer/Server/API.hs (as of ca8982b).
Return the group names and member counts for locally-hosted groups, similar to the original pioneer thread, but in json format.
What we mean here is to obtain a valid group token first, right? The corresponding thread in this PR is pretty close, it is just a single poke with empty token (that will trigger generation of a new one if no token is found). |
Yeah, the thread as-is is no good, and you can't trivially do it using plain API calls, because hosting cares about learning the invite token at the end. |
Matches the behavior pioneer used to hand-roll.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fc8033be5
ℹ️ 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".
Pioneer-side does these over HTTP now.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 200bd34050
ℹ️ 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".
| (~(put ju hosts) p.q.nest flag) | ||
| :: | ||
| %- pure:m | ||
| !> ^- json |
There was a problem hiding this comment.
probably should break this one up kinda hard to read
mikolajpp
left a comment
There was a problem hiding this comment.
Overall this looks quite good.
There is a question of how hosting handles thread crashes. If these are monitored and acted upon, I think we are good. If not, we can think about making these threads slightly more resilient, especially in places where we scry without checking if agent is running.
There are also various styling nits to be addressed, but nothing major.
The only blocker seems to be lure issues already flagged by @arthyn
| :: shared aqua boot from /ted/ph/test brings up ~zod, ~bud, ~nec, ~fen, ~dem | ||
| :: with the %groups desk synced and ~fen configured as lure provider. | ||
| :: | ||
| :: to run all of these: | ||
| :: ./backend/run-aqua-tests.sh | ||
| :: | ||
| :: to run a specific subset, pass an arm-name pattern as the trailing | ||
| :: path component to %ph-test (the test runner uses it as a prefix filter). |
There was a problem hiding this comment.
I assume these instructions are for agents? It's not really the place to keep them here, we have more than enough information in docs/aqua in our repository.
| ;< out=(unit ?) bind:m (scry-aqua (unit ?) who pax) | ||
| (pure:m (fall out |)) | ||
| :: | ||
| :: ─── tests ─────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
I would avoid such embellishments on principle to keep style consistent in aqua tests and avoid confusing agents. We know where tests start by simply searching for ++ ph.
| =/ m (strand ,vase) | ||
| ^- form:m | ||
| ;< =groups:v9:gv bind:m | ||
| (scry groups:v9:gv /gx/groups/v2/groups/groups-2) |
There was a problem hiding this comment.
| (scry groups:v9:gv /gx/groups/v2/groups/groups-2) | |
| (scry groups:v9:gv %gx /groups/v2/groups/groups-2) |
| =/ hosts=(jug ship flag:v9:gv) | ||
| %+ roll ~(tap by groups) | ||
| |= [[=flag:v9:gv =group:v9:gv] hosts=(jug ship flag:v9:gv)] | ||
| %+ roll ~(tap by channels.group) | ||
| |= [[=nest:v9:gv *] =_hosts] | ||
| ?. ?=(%chat p.nest) hosts | ||
| (~(put ju hosts) p.q.nest flag) |
There was a problem hiding this comment.
Iterating with these two loops we will have already seen every group, host and channel. If we record all this information accordingly in something like hosts=(map ship (jar flag @ta)) then this would correspond directly to the serialized result and we would not need to filter below.
There was a problem hiding this comment.
I considered this originally but couldn't make it work in a way that didn't feel complex. Maybe I'll revisit one more time.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6ec289047
ℹ️ 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".
| =/ oldest=(may:v7:cv writ:v7:cv) +.i.writs-list | ||
| ?. ?=(%& -.oldest) (pure:m !>(empty-out)) |
There was a problem hiding this comment.
Skip tombstones when deleting the oldest DM
When the oldest entry in the paged result is a tombstone, this returns deleted:false immediately. A normal use case hits this after the thread has deleted one message already, because %del leaves a tombstone in the DM writs map (desk/lib/dm.hoon), so the next call sees that tombstone first and stops even though later live messages remain; page forward until a %& writ is found before reporting the DM empty.
Useful? React with 👍 / 👎.
| =/ arg=^json | ||
| %- pairs:enjs:format | ||
| :~ id+s+'pioneer-personal' | ||
| tag+s+'personal' | ||
| == |
There was a problem hiding this comment.
Pass a string to create-lure-invite in the test
This test builds an object with id and tag, but pioneer/create-lure-invite parses its entire argument with so:dejs:format, so the thread expects the JSON payload itself to be a string. As written, this test fails during argument parsing before it can exercise invite generation; pass s+'pioneer-personal' (or update the thread parser) instead.
Useful? React with 👍 / 👎.
Can just use group-create-1 for this.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 030e20f3d0
ℹ️ 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".
| ;< exists=? bind:m | ||
| (scry ? /gu/channels/v1/chat/(scot %p our.bowl)/[name.args]) |
There was a problem hiding this comment.
Use a registered channel scry for idempotency
When this thread is retried for an existing channel, this %gu lookup is against /v1/chat/..., but the channels agent only registers channel scries under the /x/... routes such as /x/v5/channels, posts, heads, etc. (checked desk/app/channels.hoon:172-230), not a single-channel /v1/chat route. That makes exists stay false, so the retry falls through to %create and hits ca-create's existing-nest assertion (desk/app/channels.hoon:1915-1916) instead of returning created:false.
Useful? React with 👍 / 👎.
| description+s+'created by pioneer thread test' | ||
| == | ||
| :: first invocation: should create the group | ||
| ;< ~ bind:m (run-thread-ok ~zod %create-private-group arg) |
There was a problem hiding this comment.
Point the private-group test at an existing thread
This test invokes pioneer-create-private-group, but this commit does not add create-private-group.hoon and a repo search of the reviewed tree under desk/ted/pioneer only finds the seven newly added threads. As a result Spider fails to load the thread before the test can exercise idempotent group creation, so either add that thread or change the test to call an existing one.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dca1528be
ℹ️ 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".
| =/ who (author-of w) | ||
| ?: =(who our.bowl) [+(sent) recv] | ||
| [sent +(recv)] |
There was a problem hiding this comment.
Skip tombstones before counting DM messages
When a DM contains deleted messages, /writs/newest/.../light still returns %| tombstones in the (may writ) set, and this loop feeds them through author-of and increments sent/received anyway. That inflates the reported message counts after deletions; filter for %& live writs before deriving the author or counting the row.
Useful? React with 👍 / 👎.
The thread api was changed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 875867a1a8
ℹ️ 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".
mikolajpp
left a comment
There was a problem hiding this comment.
This looks good. Thanks for the styling adjustements! I assume these are going to be, or were already tested off-band, since we have only a scant test coverage here. It is good to go as is though; we can generate remaining tests later under 408k.
There was a problem hiding this comment.
i had to make some changes to this to get it to work with pioneer; mainly, spider passes json thread input as (unit json), not raw json, so i changed the thread args. (also note im just pasting the diff into a codeblock here, i haven't pushed to this branch)
also:
- some scries needed mark suffixes
create-group-lure-invitenow parses flag first and reads title/image only if present to avoid crashes
diff --git a/desk/ted/pioneer/create-chat-channel.hoon b/desk/ted/pioneer/create-chat-channel.hoon
index 0699adcc2..72467cd4f 100644
--- a/desk/ted/pioneer/create-chat-channel.hoon
+++ b/desk/ted/pioneer/create-chat-channel.hoon
@@ -20,7 +20,9 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ args=[=flag:g name=@tas title=@t description=(unit @t) image=(unit @t)]
%. json
%- ot
diff --git a/desk/ted/pioneer/create-group-lure-invite.hoon b/desk/ted/pioneer/create-group-lure-invite.hoon
index 8077ad455..13243fe45 100644
--- a/desk/ted/pioneer/create-group-lure-invite.hoon
+++ b/desk/ted/pioneer/create-group-lure-invite.hoon
@@ -20,23 +20,31 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl:io
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ parse-args
=, dejs:format
- %- ot
- :~ flag+flag:dejs:gj
- title+(mu so)
- image+(mu so)
- ==
-=/ args=[=flag:g title=(unit @t) image=(unit @t)] (parse-args json)
+ (ot ~[flag+flag:dejs:gj])
+=/ args=[=flag:g] (parse-args json)
=* flag flag.args
+=/ maybe-field
+ |= key=@t
+ ^- (unit @t)
+ ?. ?=([%o *] json) ~
+ =/ value=(unit ^json) (~(get by p.json) key)
+ ?~ value ~
+ ?. ?=([%s *] u.value) ~
+ [~ p.u.value]
+=/ title=(unit @t) (maybe-field 'title')
+=/ image=(unit @t) (maybe-field 'image')
=/ flag-str=cord (crip "{<p.flag>}/{(trip q.flag)}")
=/ fields=(map field:reel cord)
=/ acc=(map field:reel cord) [[%'inviteType' 'group'] ~ ~]
- =? acc ?=(^ title.args)
- (~(put by acc) %'invitedGroupTitle' u.title.args)
- =? acc ?=(^ image.args)
- (~(put by acc) %'invitedGroupIconImageUrl' u.image.args)
+ =? acc ?=(^ title)
+ (~(put by acc) %'invitedGroupTitle' u.title)
+ =? acc ?=(^ image)
+ (~(put by acc) %'invitedGroupIconImageUrl' u.image)
acc
=/ =metadata:v1:reel [%groups-0 fields]
=/ =wire /group-lure-invite/(scot %da now.bowl)
diff --git a/desk/ted/pioneer/create-lure-invite.hoon b/desk/ted/pioneer/create-lure-invite.hoon
index b31c6dfd5..3d73ac2d6 100644
--- a/desk/ted/pioneer/create-lure-invite.hoon
+++ b/desk/ted/pioneer/create-lure-invite.hoon
@@ -17,7 +17,9 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl:io
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ id=cord (so:dejs:format json)
=/ =wire /lure-invite/(scot %da now.bowl)
;< ~ bind:m
diff --git a/desk/ted/pioneer/get-dm-counts.hoon b/desk/ted/pioneer/get-dm-counts.hoon
index e6b86447b..c3ba78176 100644
--- a/desk/ted/pioneer/get-dm-counts.hoon
+++ b/desk/ted/pioneer/get-dm-counts.hoon
@@ -23,7 +23,9 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ args=[target=ship limit=(unit @ud)]
((ot ship+(se %p) limit+(mu ni) ~) json)
=/ limit=@ud (fall limit.args 1.000)
@@ -31,16 +33,13 @@
/v4/dm/(scot %p target.args)/writs/newest/(scot %ud limit)/light/chat-paged-writs-4
;< =paged-writs:v7:cv bind:m (scry paged-writs:v7:cv %gx %chat path)
=/ writs-list (tap:on:writs:v7:cv writs.paged-writs)
-:: extract author ship from each writ; treats bot-author entries as their ship
-::
-=/ author-of
- |= w=(may:v7:cv writ:v7:cv)
- ^- ship
- =/ a=author:v7:cv ?:(?=(%| -.w) author.w author.w)
- ?@(a a ship.a)
=+ %+ roll writs-list
|= [[=time w=(may:v7:cv writ:v7:cv)] sent=@ud recv=@ud]
- =/ who (author-of w)
+ ?: ?=(%| -.w)
+ [sent recv]
+ =/ who=ship
+ =/ a=author:v7:cv author.w
+ ?@(a a ship.a)
?: =(who our.bowl) [+(sent) recv]
[sent +(recv)]
=/ out=^json
diff --git a/desk/ted/pioneer/is-agent-running.hoon b/desk/ted/pioneer/is-agent-running.hoon
index fe29ac1fa..3f02181d6 100644
--- a/desk/ted/pioneer/is-agent-running.hoon
+++ b/desk/ted/pioneer/is-agent-running.hoon
@@ -14,7 +14,9 @@
|= arg=vase
=/ m (strand ,vase)
^- form:m
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ app=@tas ((ot agent+(se %tas) ~) json)
;< running=? bind:m (scry ? /gu/[app]/$)
-(pure:m !>(`json`b+running))
+(pure:m !>(b+running))
diff --git a/desk/ted/pioneer/set-group-title.hoon b/desk/ted/pioneer/set-group-title.hoon
index 5908ff721..9cfa4b7f3 100644
--- a/desk/ted/pioneer/set-group-title.hoon
+++ b/desk/ted/pioneer/set-group-title.hoon
@@ -14,7 +14,9 @@
=/ m (strand ,vase)
^- form:m
;< our=@p bind:m get-our
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ args=[=flag:g title=@t]
((ot flag+flag:dejs:gj title+so ~) json)
=* flag flag.args
diff --git a/desk/tests/ph/for/pioneer.hoon b/desk/tests/ph/for/pioneer.hoon
index 5dbada067..58a6d3ac0 100644
--- a/desk/tests/ph/for/pioneer.hoon
+++ b/desk/tests/ph/for/pioneer.hoon
@@ -22,7 +22,7 @@
(cat 3 name (cat 3 '-' (scot %uv (sham now.bowl name))))
=/ =beak [who %groups da+now.bowl]
=/ file=term (cat 3 'pioneer-' name)
- =/ start=start-args:spider [~ `tid beak file !>(arg)]
+ =/ start=start-args:spider [~ `tid beak file !>(`(unit json)`[~ arg])]
=/ watch-path=path /(scot %p who)/spider/thread-result/[tid]
;< ~ bind:m (watch-app watch-path [who %spider] /thread-result/[tid])
;< ~ bind:m (poke-app [who %spider] spider-start+start)
➜ tlon-apps git:(hm/hosting-threads) ✗ git diff
➜ tlon-apps git:(hm/hosting-threads) ✗ git diff > pr.diff
➜ tlon-apps git:(hm/hosting-threads) ✗ cat pr.diff
diff --git a/desk/ted/pioneer/create-chat-channel.hoon b/desk/ted/pioneer/create-chat-channel.hoon
index 0699adcc2..72467cd4f 100644
--- a/desk/ted/pioneer/create-chat-channel.hoon
+++ b/desk/ted/pioneer/create-chat-channel.hoon
@@ -20,7 +20,9 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ args=[=flag:g name=@tas title=@t description=(unit @t) image=(unit @t)]
%. json
%- ot
diff --git a/desk/ted/pioneer/create-group-lure-invite.hoon b/desk/ted/pioneer/create-group-lure-invite.hoon
index 8077ad455..13243fe45 100644
--- a/desk/ted/pioneer/create-group-lure-invite.hoon
+++ b/desk/ted/pioneer/create-group-lure-invite.hoon
@@ -20,23 +20,31 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl:io
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ parse-args
=, dejs:format
- %- ot
- :~ flag+flag:dejs:gj
- title+(mu so)
- image+(mu so)
- ==
-=/ args=[=flag:g title=(unit @t) image=(unit @t)] (parse-args json)
+ (ot ~[flag+flag:dejs:gj])
+=/ args=[=flag:g] (parse-args json)
=* flag flag.args
+=/ maybe-field
+ |= key=@t
+ ^- (unit @t)
+ ?. ?=([%o *] json) ~
+ =/ value=(unit ^json) (~(get by p.json) key)
+ ?~ value ~
+ ?. ?=([%s *] u.value) ~
+ [~ p.u.value]
+=/ title=(unit @t) (maybe-field 'title')
+=/ image=(unit @t) (maybe-field 'image')
=/ flag-str=cord (crip "{<p.flag>}/{(trip q.flag)}")
=/ fields=(map field:reel cord)
=/ acc=(map field:reel cord) [[%'inviteType' 'group'] ~ ~]
- =? acc ?=(^ title.args)
- (~(put by acc) %'invitedGroupTitle' u.title.args)
- =? acc ?=(^ image.args)
- (~(put by acc) %'invitedGroupIconImageUrl' u.image.args)
+ =? acc ?=(^ title)
+ (~(put by acc) %'invitedGroupTitle' u.title)
+ =? acc ?=(^ image)
+ (~(put by acc) %'invitedGroupIconImageUrl' u.image)
acc
=/ =metadata:v1:reel [%groups-0 fields]
=/ =wire /group-lure-invite/(scot %da now.bowl)
diff --git a/desk/ted/pioneer/create-lure-invite.hoon b/desk/ted/pioneer/create-lure-invite.hoon
index b31c6dfd5..3d73ac2d6 100644
--- a/desk/ted/pioneer/create-lure-invite.hoon
+++ b/desk/ted/pioneer/create-lure-invite.hoon
@@ -17,7 +17,9 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl:io
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ id=cord (so:dejs:format json)
=/ =wire /lure-invite/(scot %da now.bowl)
;< ~ bind:m
diff --git a/desk/ted/pioneer/get-dm-counts.hoon b/desk/ted/pioneer/get-dm-counts.hoon
index e6b86447b..b5b6f4ecb 100644
--- a/desk/ted/pioneer/get-dm-counts.hoon
+++ b/desk/ted/pioneer/get-dm-counts.hoon
@@ -23,7 +23,9 @@
=/ m (strand ,vase)
^- form:m
;< =bowl:spider bind:m get-bowl
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ args=[target=ship limit=(unit @ud)]
((ot ship+(se %p) limit+(mu ni) ~) json)
=/ limit=@ud (fall limit.args 1.000)
diff --git a/desk/ted/pioneer/is-agent-running.hoon b/desk/ted/pioneer/is-agent-running.hoon
index fe29ac1fa..3f02181d6 100644
--- a/desk/ted/pioneer/is-agent-running.hoon
+++ b/desk/ted/pioneer/is-agent-running.hoon
@@ -14,7 +14,9 @@
|= arg=vase
=/ m (strand ,vase)
^- form:m
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ app=@tas ((ot agent+(se %tas) ~) json)
;< running=? bind:m (scry ? /gu/[app]/$)
-(pure:m !>(`json`b+running))
+(pure:m !>(b+running))
diff --git a/desk/ted/pioneer/set-group-title.hoon b/desk/ted/pioneer/set-group-title.hoon
index 5908ff721..9cfa4b7f3 100644
--- a/desk/ted/pioneer/set-group-title.hoon
+++ b/desk/ted/pioneer/set-group-title.hoon
@@ -14,7 +14,9 @@
=/ m (strand ,vase)
^- form:m
;< our=@p bind:m get-our
-=+ !<(=json arg)
+=+ !<(arg=(unit json) arg)
+?> ?=(^ arg)
+=* json u.arg
=/ args=[=flag:g title=@t]
((ot flag+flag:dejs:gj title+so ~) json)
=* flag flag.args
diff --git a/desk/tests/ph/for/pioneer.hoon b/desk/tests/ph/for/pioneer.hoon
index 5dbada067..58a6d3ac0 100644
--- a/desk/tests/ph/for/pioneer.hoon
+++ b/desk/tests/ph/for/pioneer.hoon
@@ -22,7 +22,7 @@
(cat 3 name (cat 3 '-' (scot %uv (sham now.bowl name))))
=/ =beak [who %groups da+now.bowl]
=/ file=term (cat 3 'pioneer-' name)
- =/ start=start-args:spider [~ `tid beak file !>(arg)]
+ =/ start=start-args:spider [~ `tid beak file !>(`(unit json)`[~ arg])]
=/ watch-path=path /(scot %p who)/spider/thread-result/[tid]
;< ~ bind:m (watch-app watch-path [who %spider] /thread-result/[tid])
;< ~ bind:m (poke-app [who %spider] spider-start+start)Co-authored-by: reid <yapishu@users.noreply.github.com>
Truly optional fields, instead of null-or-value. Co-authored-by: reid <yapishu@users.noreply.github.com>
Co-authored-by: reid <yapishu@users.noreply.github.com>
9b8b030 to
12b544c
Compare
|
Sweet, thank you. I made some small adjustments and applied your changes, much appreciated.
Huh. I ran all these threads and should've hit their scries. I also don't see anything of this sort in your patch. A quick search doesn't reveal any obvious misses here. Are we in the clear? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12b544c0ef
ℹ️ 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".
| =/ =metadata:v1:reel [%groups-0 fields] | ||
| =/ =wire /group-lure-invite/(scot %da now.bowl) | ||
| ;< ~ bind:m | ||
| (watch:io wire [our.bowl %reel] /v1/id-link/[flag-str]) |
There was a problem hiding this comment.
Use stab for slash-containing invite ids
When the group flag is used as the reel id it always contains a / (for example ~zod/test-group), but this subscription builds the watch path as a single interpolated path segment. %reel emits the confirmation on the path it builds with stab (cat 3 '/v1/id-link/' id) (desk/app/reel.hoon:277), and the existing lure test uses the same stab pattern for slash-containing ids (desk/tests/ph/for/lure.hoon:114), so this watcher is on a different path and take-fact can wait forever for normal group flags. Build the watch path with stab from flag-str before subscribing.
Useful? React with 👍 / 👎.
Early draft to track remaining work. I did a pass over all the threads @arthyn generated and cross-referenced them against ylem's
Pioneer/Server/API.hs.For now, I've only done so for overall usage. Once we have these sufficiently trimmed down we should do a pass to check them for parity with the existing Click threads and either make them match or leave warnings/instructions for hosting devs about the differences.
Here's the state of threads in this PR, broken down by status:
👍 Removed, already self-serve integrated by pioneer
🔗 Can be made redundant by simple pioneer-side integration
create-group-invite: poke with%group-action-4challenge: need to generate@uv-formatted entropy for the token/~/scry/channels/v5/[kind]/[ship]/[name]/search/text/0/1/[needle].jsonand count resulting array size (thread too general, too)/~/scry/chat/v4/dm/[ship]/search/text/0/1/[needle].jsonand count resulting array size (thread too general, too)get-channels: scry at/~/scry/channels/v5/channels.jsonchallenge: thread doesn't produce the same as thegetChatChannelsthread did.%channel-command(or local ship with%chat-action)%chat-dm-action-2%group-foreign-2☑️ Tested, but needs pioneer integration
hosted-groups / is-group-hostwant pioneer-side flexibility🔁 Needs a rewrite
🤔 Up for consideration