Skip to content

refactor(desktop): address a conversation by its session id alone #888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tonyfettes wants to merge 1 commit into from
Closed

refactor(desktop): address a conversation by its session id alone #888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions desktop/frontend/archive.mbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/// the dialog appear to authorize a different conversation.
priv struct ArchivedDeleteConfirm {
channel : @interop.ChannelId
key : ArchiveRecordKey
session : String
// The parent plus every descendant visible in the same archived snapshot.
// Host notifications still cover a child created after that snapshot.
sessions : Array[String]
Expand Down Expand Up @@ -143,14 +143,11 @@ fn unarchive_session(
fn delete_archived_session(
dispatch : @cmd.Emit[Msg],
channel : @interop.ChannelId,
key : ArchiveRecordKey,
session : String,
sessions : Array[String],
connection_generation : Int,
archived_request_generation : Int,
) -> @cmd.Cmd {
guard key.workspace_payload(channel) is Some(workspace) else {
return @cmd.none
}
let device_dispatch = dispatch.map((msg : DeviceMsg) => {
FromDevice(channel, msg)
})
Expand All @@ -159,7 +156,7 @@ fn delete_archived_session(
let reply = @interop.bridge_request(
channel,
@commands.session_delete_archived,
{ session: key.session, workspace },
{ session, },
) catch {
error => {
scheduler.add(
Expand All @@ -173,7 +170,6 @@ fn delete_archived_session(
scheduler.add(
device_dispatch(
ArchivedDeleted(
key~,
sessions~,
items=@transcript.sessions_from_reply(reply, channel),
connection_generation~,
Expand Down
15 changes: 3 additions & 12 deletions desktop/frontend/boot.mbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,8 @@ fn SidebarDispatcher::conversation_open(
}
match id.source() {
"openseek.live" => self.emit(OpenSession(channel, id.conversation()))
"openseek.archived" => {
let workspace = id.root().bind(root => @resource.of_path(channel, root))
self.emit(
OpenArchivedSession(channel, session=id.conversation(), workspace~),
)
}
"openseek.archived" =>
self.emit(OpenArchivedSession(channel, id.conversation()))
source => self.unroutable("open", "source \{source}")
}
}
Expand Down Expand Up @@ -205,12 +201,7 @@ fn SidebarDispatcher::conversation_delete(
id.source() == "openseek.archived" else {
return self.unroutable("delete", "\{id.source()}@\{id.channel()}")
}
self.emit(
DeleteArchivedSession(channel, {
session: id.conversation(),
workspace: id.root().bind(root => @resource.of_path(channel, root)),
}),
)
self.emit(DeleteArchivedSession(channel, id.conversation()))
}

///|
Expand Down
47 changes: 8 additions & 39 deletions desktop/frontend/bridge.mbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ fn device_msg(
AgentError(payload) => error_msg(payload)
AgentFinished(payload) => finished_msg(payload)
AgentDurable(payload) => durable_boundary_msg(payload)
SessionEvent(payload) => session_event_msg(channel, payload)
SessionEvent(payload) => session_event_msg(payload)
SubrunProgress(payload) =>
Some(
SubrunProgressed(
Expand All @@ -192,7 +192,7 @@ fn device_msg(
activity=payload.activity,
),
)
SessionChanged(payload) => session_changed_msg(channel, payload)
SessionChanged(payload) => session_changed_msg(payload)
WorkspaceChanged(payload) => {
let paths : Array[@common.Uri] = []
// The wire contract still calls registered projects `workspaces`.
Expand Down Expand Up @@ -226,22 +226,12 @@ fn device_msg(
/// Decode archive invalidation synchronously. A fresh id is allocated at the
/// event boundary (outside `update`) so archiving or deleting the active
/// conversation can switch without a render where Send targets the stale id.
fn session_changed_msg(
channel : @interop.ChannelId,
payload : @protocol.SessionChangedPayload,
) -> DeviceMsg? {
guard ArchiveRecordKey::from_protocol(
channel,
payload.session,
payload.workspace,
)
is Some(key) else {
return None
}
fn session_changed_msg(payload : @protocol.SessionChangedPayload) -> DeviceMsg? {
guard !payload.session.is_empty() else { return None }
Some(
SessionsChanged(
change=payload.change,
key~,
session=payload.session,
fresh_session=if payload.change == "archived" ||
payload.change == "deleted" {
Some(generated_session_id())
Expand Down Expand Up @@ -855,22 +845,14 @@ fn RecoveredSettlement::from_reply(
/// the outer notification for routing; the typed item enum is what the
/// transcript projector consumes, with raw unknown items retained for newer
/// Desktop versions.
fn session_event_msg(
channel : @interop.ChannelId,
payload : @protocol.SessionCommitPayload,
) -> DeviceMsg? {
// The store root is half of the commit's durable identity; an event whose
// root does not bind to the channel is malformed and dropped.
guard @resource.of_path(channel, payload.session_root) is Some(session_root) else {
return None
}
fn session_event_msg(payload : @protocol.SessionCommitPayload) -> DeviceMsg? {
guard !payload.session.is_empty() else { return None }
Some(
SessionCommitted(
session=payload.session,
sequence=payload.sequence,
ts=payload.event.ts,
item=payload.event.item,
session_root~,
),
)
}
Expand Down Expand Up @@ -1150,21 +1132,18 @@ async fn refresh_sessions_into(

///|
/// Load a durable session's transcript so the conversation can be resumed.
/// `workspace` says which registered store holds it; `None` selects the
/// default store. The protocol conversion happens only in `load_session_cmd`.
/// The id alone addresses the record; the host locates the store holding it.
fn open_session(
dispatch : @cmd.Emit[Msg],
channel : @interop.ChannelId,
session_id : String,
workspace~ : @common.Uri?,
generation~ : Int,
archived? : Bool = false,
) -> @cmd.Cmd {
let dispatch = dispatch.map((msg : DeviceMsg) => FromDevice(channel, msg))
load_session_cmd(
channel,
session_id,
workspace~,
archived~,
on_loaded=view => {
dispatch(
Expand Down Expand Up @@ -1203,15 +1182,13 @@ fn refresh_session(
dispatch : @cmd.Emit[Msg],
channel : @interop.ChannelId,
session_id : String,
workspace~ : @common.Uri?,
generation~ : Int,
archived? : Bool = false,
) -> @cmd.Cmd {
let dispatch = dispatch.map((msg : DeviceMsg) => FromDevice(channel, msg))
load_session_cmd(
channel,
session_id,
workspace~,
archived~,
on_loaded=view => {
dispatch(
Expand All @@ -1236,28 +1213,20 @@ fn refresh_session(
fn load_session_cmd(
channel : @interop.ChannelId,
session_id : String,
workspace~ : @common.Uri?,
archived~ : Bool,
on_loaded~ : (@transcript.SessionLoadView) -> @cmd.Cmd,
on_failed~ : (String) -> @cmd.Cmd,
) -> @cmd.Cmd {
match workspace {
Some(resource) if !@resource.belongs_to(resource, channel) =>
return @cmd.none
_ => ()
}
@cmd.custom_cmd(scheduler => {
@js.async_run(() => {
let reply = try {
if archived {
@interop.bridge_request(channel, @commands.session_load_archived, {
session: session_id,
workspace: workspace.map(path => path.path),
})
} else {
@interop.bridge_request(channel, @commands.session_load, {
session: session_id,
workspace: workspace.map(path => path.path),
})
}
} catch {
Expand Down
17 changes: 3 additions & 14 deletions desktop/frontend/goal.mbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ test "only the matching set marker settles the pending text" {
sequence~,
ts=None,
item=@protocol.RuntimeNotice({ content, }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down Expand Up @@ -293,7 +292,6 @@ test "a clear waits for its tombstone, not for any goal marker" {
sequence~,
ts=None,
item=@protocol.RuntimeNotice({ content, }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down Expand Up @@ -357,7 +355,6 @@ test "goal marker commits write the mirror and render cards" {
sequence=1,
ts=None,
item=@protocol.RuntimeNotice({ content: "[goal]\nship the feature" }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
test_model(),
)
Expand All @@ -376,7 +373,6 @@ test "goal marker commits write the mirror and render cards" {
item=@protocol.RuntimeNotice({
content: "[goal blocked]\nneeds a decision",
}),
session_root=@interop.ChannelId::Local.test_store_root(),
),
set,
)
Expand All @@ -391,7 +387,6 @@ test "goal marker commits write the mirror and render cards" {
sequence=3,
ts=None,
item=@protocol.RuntimeNotice({ content: "[goal cleared]" }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
blocked,
)
Expand Down Expand Up @@ -544,7 +539,6 @@ test "a clear over an unconfirmed set waits for a goal to actually go away" {
sequence~,
ts=None,
item=@protocol.RuntimeNotice({ content, }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down Expand Up @@ -584,7 +578,6 @@ test "a tombstone over an empty mirror still ends the clear" {
sequence=1,
ts=None,
item=@protocol.RuntimeNotice({ content: "[goal cleared]" }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down Expand Up @@ -613,7 +606,6 @@ test "re-asserting an unchanged goal waits for its own marker too" {
sequence~,
ts=None,
item=@protocol.RuntimeNotice({ content, }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down Expand Up @@ -727,7 +719,6 @@ test "re-asserting a blocked goal waits for its own marker" {
sequence~,
ts=None,
item=@protocol.RuntimeNotice({ content, }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down Expand Up @@ -765,7 +756,7 @@ test "archiving carries the goal draft to the replacement" {
dispatch,
SessionsChanged(
change="archived",
key={ session: "desktop-test", workspace: None },
session="desktop-test",
fresh_session=Some("desktop-restored"),
),
model,
Expand Down Expand Up @@ -795,7 +786,7 @@ test "archiving carries a goal that was sent but never confirmed" {
dispatch,
SessionsChanged(
change="archived",
key={ session: "desktop-test", workspace: None },
session="desktop-test",
fresh_session=Some("desktop-restored"),
),
model,
Expand Down Expand Up @@ -824,7 +815,7 @@ test "archiving carries no goal the record already confirmed" {
dispatch,
SessionsChanged(
change="archived",
key={ session: "desktop-test", workspace: None },
session="desktop-test",
fresh_session=Some("desktop-restored"),
),
model,
Expand Down Expand Up @@ -1077,7 +1068,6 @@ test "an undelivered reply goes quiet once its send has been settled" {
sequence=1,
ts=None,
item=@protocol.RuntimeNotice({ content: "[goal]\nship the feature" }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down Expand Up @@ -1137,7 +1127,6 @@ test "a rejection that arrives after the record has spoken only reports" {
sequence=1,
ts=None,
item=@protocol.RuntimeNotice({ content: "[goal]\nthe goal that won" }),
session_root=@interop.ChannelId::Local.test_store_root(),
),
model,
)
Expand Down
Loading