mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 00:57:45 +00:00
fix(mobile): stop review notes from disappearing or coming back after deletion (#31699)
* fix(mobile): stop review notes disappearing or coming back after deletion The phone sent its whole notes list on every add or delete, and the host stored it as is. A note added on desktop after the phone loaded was wiped by the phone's next edit, two quick taps could overwrite each other, and a failed save rolled back to an old snapshot. worktree.set now takes an optional diffCommentsBase, like linkedItemsBase: the host applies only the writer's own adds, edits and removals to the stored notes. The phone sends the list it started from, shows the host's merged list from the reply, and undoes only its own change on failure. Older hosts drop the field and replace the list as before; older phones send no base and get the same. Co-authored-by: Neil <neil@stably.ai> * fix(mobile): keep loaded review notes when an early save fails The first load was discarded once any save started, so a save that failed before it landed left the host's notes hidden. The load now applies the phone's changes made during the read onto the host's list instead. * fix(mobile): send review-note requests one at a time Two in-flight saves could reach the host in either order, so adopting the later reply hid a note the earlier save had stored. Session notes now send one request at a time and reconcile every reply with the phone's changes made since it was sent, replacing the sequence counters. * fix(mobile): drop review-note saves queued behind a failed one A save queued behind one that failed still sent the list built on the failed change, and an older host stores that list whole. Those saves now undo their own change instead of sending. Also renames the shared base type so it no longer shares a name with the renderer's mutation record. * fix(mobile): show review notes as the host's list plus pending saves A queued reply was reconciled against the list captured at tap time, so a note desktop deleted meanwhile came back on the phone. The phone now keeps the host's last list and its unanswered saves per workspace and shows the two combined; a reply for another workspace is ignored. The review screen also shows the host's list after a save, so an edit to a note desktop deleted no longer looks saved. * fix(mobile): fail queued review-note requests while disconnected A notes request queued behind a pending load waited for a reconnect with no timeout, holding every later save. Notes requests now fail at once when the connection is down, like terminal input. * fix(mobile): drop every review-note save built on a failed or dropped one A save made while a dropped save was still pending carried its change, so an older host could store it. Each failed or dropped save now moves the failure count. * fix(mobile): send each review-note save as the host's list plus its change Each queued save now builds its payload when it is sent, from the host's last list plus only its own change. A note added before the first load no longer replaces the loaded notes on an older host, and a failed save's change never rides along, so later saves no longer need dropping. Delivered-note cleanup waits for a reconnect again so those notes are not sent twice. --------- Co-authored-by: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com>
This commit is contained in:
Neil
Kelvin Amoaba
@@ -4,7 +4,7 @@
|
||||
"family": "session.diff-notes",
|
||||
"namedDeltas": [],
|
||||
"values": {
|
||||
"106579ec0285": {
|
||||
"194b8cd56a74": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -21,186 +21,7 @@
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"id": "frame-1",
|
||||
"ok": true,
|
||||
"result": {
|
||||
"$rpc": "null"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"12a0229a2392": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"error": {
|
||||
"code": "method_not_found",
|
||||
"message": "Unknown method"
|
||||
},
|
||||
"id": "frame-1",
|
||||
"ok": false
|
||||
}
|
||||
}
|
||||
},
|
||||
"1d721a77bb40": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "rejected",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"error": {
|
||||
"category": "Error",
|
||||
"message": "",
|
||||
"isRpcDeliveryUnknown": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"270b4ca66aa7": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.show\",\"params\":{\"worktree\":\"id:workspace-1\"}}"
|
||||
},
|
||||
"432aeb4f1709": {
|
||||
"busy": false,
|
||||
"diffComments": [],
|
||||
"pendingDelivery": {
|
||||
"$rpc": "null"
|
||||
}
|
||||
},
|
||||
"48a2db1ee584": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"id": "frame-1",
|
||||
"ok": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"4a2a4280302f": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"error": {
|
||||
"code": "refused",
|
||||
"message": "outer refused"
|
||||
},
|
||||
"id": "frame-1",
|
||||
"ok": false
|
||||
}
|
||||
}
|
||||
},
|
||||
"542d7b8c3e3f": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -228,7 +49,7 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"6d49118108f1": {
|
||||
"1b52e707bbff": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -245,22 +66,26 @@
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "rejected",
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"error": {
|
||||
"category": "Error",
|
||||
"message": "transport failure",
|
||||
"isRpcDeliveryUnknown": true
|
||||
"value": {
|
||||
"id": "frame-1",
|
||||
"ok": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"7ceace53bbee": {
|
||||
"270b4ca66aa7": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.show\",\"params\":{\"worktree\":\"id:workspace-1\"}}"
|
||||
},
|
||||
"3431fb3e9b93": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -277,7 +102,117 @@
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"id": "frame-1",
|
||||
"ok": true,
|
||||
"result": {
|
||||
"error": "refused"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"432aeb4f1709": {
|
||||
"busy": false,
|
||||
"diffComments": [],
|
||||
"pendingDelivery": {
|
||||
"$rpc": "null"
|
||||
}
|
||||
},
|
||||
"6da927cd4a9e": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"error": {
|
||||
"code": "method_not_found",
|
||||
"message": "Unknown method"
|
||||
},
|
||||
"id": "frame-1",
|
||||
"ok": false
|
||||
}
|
||||
}
|
||||
},
|
||||
"6dd7c7f7521e": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"id": "frame-1",
|
||||
"ok": true,
|
||||
"result": {
|
||||
"$rpc": "null"
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"73d8b290002f": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -297,7 +232,7 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"86192c11a384": {
|
||||
"7eb43931e73b": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -314,42 +249,7 @@
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"error": {
|
||||
"code": "refused",
|
||||
"message": ""
|
||||
},
|
||||
"id": "frame-1",
|
||||
"ok": false
|
||||
}
|
||||
}
|
||||
},
|
||||
"d5df7c9e19e0": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -367,6 +267,105 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"7eec72fc398d": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "rejected",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"error": {
|
||||
"category": "Error",
|
||||
"message": "transport failure",
|
||||
"isRpcDeliveryUnknown": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"cc16a5251bdf": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "rejected",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"error": {
|
||||
"category": "Error",
|
||||
"message": "",
|
||||
"isRpcDeliveryUnknown": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"d6e940d7f092": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
{
|
||||
"name": "method",
|
||||
"value": "worktree.show"
|
||||
},
|
||||
{
|
||||
"name": "params",
|
||||
"value": {
|
||||
"worktree": "id:workspace-1"
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
"settlement": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"error": {
|
||||
"code": "refused",
|
||||
"message": ""
|
||||
},
|
||||
"id": "frame-1",
|
||||
"ok": false
|
||||
}
|
||||
}
|
||||
},
|
||||
"e28b1ad79121": {
|
||||
"busy": false,
|
||||
"diffComments": [
|
||||
@@ -414,7 +413,7 @@
|
||||
"$rpc": "undefined"
|
||||
}
|
||||
},
|
||||
"f89d7fde99a2": {
|
||||
"ff8408e804b0": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -431,7 +430,7 @@
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -440,11 +439,12 @@
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"error": {
|
||||
"code": "refused",
|
||||
"message": "outer refused"
|
||||
},
|
||||
"id": "frame-1",
|
||||
"ok": true,
|
||||
"result": {
|
||||
"error": "refused"
|
||||
}
|
||||
"ok": false
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -455,7 +455,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.normal:loaded",
|
||||
"observation": {
|
||||
"sender": ["542d7b8c3e3f"],
|
||||
"sender": ["194b8cd56a74"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -467,7 +467,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.result-absent:loaded",
|
||||
"observation": {
|
||||
"sender": ["48a2db1ee584"],
|
||||
"sender": ["1b52e707bbff"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -479,7 +479,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.result-null:loaded",
|
||||
"observation": {
|
||||
"sender": ["106579ec0285"],
|
||||
"sender": ["6dd7c7f7521e"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -491,7 +491,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.inner-ok-missing:loaded",
|
||||
"observation": {
|
||||
"sender": ["f89d7fde99a2"],
|
||||
"sender": ["3431fb3e9b93"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -503,7 +503,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.inner-false-string-error:loaded",
|
||||
"observation": {
|
||||
"sender": ["d5df7c9e19e0"],
|
||||
"sender": ["7eb43931e73b"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -515,7 +515,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.inner-false-object-error:loaded",
|
||||
"observation": {
|
||||
"sender": ["7ceace53bbee"],
|
||||
"sender": ["73d8b290002f"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -527,7 +527,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.outer-refused:loaded",
|
||||
"observation": {
|
||||
"sender": ["4a2a4280302f"],
|
||||
"sender": ["ff8408e804b0"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -539,7 +539,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.outer-refused-no-message:loaded",
|
||||
"observation": {
|
||||
"sender": ["86192c11a384"],
|
||||
"sender": ["d6e940d7f092"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -551,7 +551,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.method-not-found:loaded",
|
||||
"observation": {
|
||||
"sender": ["12a0229a2392"],
|
||||
"sender": ["6da927cd4a9e"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -563,7 +563,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.transport-rejection:loaded",
|
||||
"observation": {
|
||||
"sender": ["6d49118108f1"],
|
||||
"sender": ["7eec72fc398d"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
@@ -575,7 +575,7 @@
|
||||
{
|
||||
"id": "session-diff-notes-loaded.transport-rejection-no-message:loaded",
|
||||
"observation": {
|
||||
"sender": ["1d721a77bb40"],
|
||||
"sender": ["cc16a5251bdf"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
|
||||
+631
-510
File diff suppressed because it is too large
Load Diff
@@ -56,7 +56,20 @@
|
||||
"$rpc": "null"
|
||||
}
|
||||
},
|
||||
"b19741dc6694": {
|
||||
"81c9edeaad86": {
|
||||
"name": "worktree.set#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.set\",\"params\":{\"worktree\":\"id:workspace-1\",\"diffComments\":[{\"side\":\"modified\",\"id\":\"note-1\",\"worktreeId\":\"workspace-1\",\"filePath\":\"src/app.ts\",\"lineNumber\":4,\"body\":\"needs a test\",\"createdAt\":0}],\"diffCommentsBase\":[{\"side\":\"modified\",\"id\":\"note-1\",\"worktreeId\":\"workspace-1\",\"filePath\":\"src/app.ts\",\"lineNumber\":4,\"body\":\"needs a test\",\"createdAt\":0}],\"mobileDiffReview\":{\"version\":1,\"files\":{\"unstaged:src/app.ts\":{\"key\":\"unstaged:src/app.ts\",\"filePath\":\"src/app.ts\",\"scope\":\"unstaged\",\"lastSeenDiffIdentity\":\"identity-1\",\"reviewedAt\":1767225600000,\"reviewDiffIdentity\":\"identity-1\"}},\"updatedAt\":1767225600000,\"completedAt\":1767225600000}}}"
|
||||
},
|
||||
"eb79a9b3682a": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"$rpc": "undefined"
|
||||
}
|
||||
},
|
||||
"ebfe5f82347b": {
|
||||
"name": "worktree.set#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -78,6 +91,17 @@
|
||||
"worktreeId": "workspace-1"
|
||||
}
|
||||
],
|
||||
"diffCommentsBase": [
|
||||
{
|
||||
"body": "needs a test",
|
||||
"createdAt": 0,
|
||||
"filePath": "src/app.ts",
|
||||
"id": "note-1",
|
||||
"lineNumber": 4,
|
||||
"side": "modified",
|
||||
"worktreeId": "workspace-1"
|
||||
}
|
||||
],
|
||||
"mobileDiffReview": {
|
||||
"completedAt": 1767225600000,
|
||||
"files": {
|
||||
@@ -121,19 +145,6 @@
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"bc47263d2197": {
|
||||
"name": "worktree.set#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.set\",\"params\":{\"worktree\":\"id:workspace-1\",\"diffComments\":[{\"side\":\"modified\",\"id\":\"note-1\",\"worktreeId\":\"workspace-1\",\"filePath\":\"src/app.ts\",\"lineNumber\":4,\"body\":\"needs a test\",\"createdAt\":0}],\"mobileDiffReview\":{\"version\":1,\"files\":{\"unstaged:src/app.ts\":{\"key\":\"unstaged:src/app.ts\",\"filePath\":\"src/app.ts\",\"scope\":\"unstaged\",\"lastSeenDiffIdentity\":\"identity-1\",\"reviewedAt\":1767225600000,\"reviewDiffIdentity\":\"identity-1\"}},\"updatedAt\":1767225600000,\"completedAt\":1767225600000}}}"
|
||||
},
|
||||
"eb79a9b3682a": {
|
||||
"status": "fulfilled",
|
||||
"startedAt": 0,
|
||||
"settledAt": 0,
|
||||
"value": {
|
||||
"$rpc": "undefined"
|
||||
}
|
||||
}
|
||||
},
|
||||
"recording": {
|
||||
@@ -142,8 +153,8 @@
|
||||
{
|
||||
"id": "persisted",
|
||||
"observation": {
|
||||
"sender": ["b19741dc6694"],
|
||||
"payloads": ["bc47263d2197"],
|
||||
"sender": ["ebfe5f82347b"],
|
||||
"payloads": ["81c9edeaad86"],
|
||||
"settlements": {
|
||||
"mark-reviewed": "eb79a9b3682a"
|
||||
},
|
||||
|
||||
@@ -37,7 +37,12 @@
|
||||
"$rpc": "null"
|
||||
}
|
||||
},
|
||||
"ac014f21aa55": {
|
||||
"81c9edeaad86": {
|
||||
"name": "worktree.set#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.set\",\"params\":{\"worktree\":\"id:workspace-1\",\"diffComments\":[{\"side\":\"modified\",\"id\":\"note-1\",\"worktreeId\":\"workspace-1\",\"filePath\":\"src/app.ts\",\"lineNumber\":4,\"body\":\"needs a test\",\"createdAt\":0}],\"diffCommentsBase\":[{\"side\":\"modified\",\"id\":\"note-1\",\"worktreeId\":\"workspace-1\",\"filePath\":\"src/app.ts\",\"lineNumber\":4,\"body\":\"needs a test\",\"createdAt\":0}],\"mobileDiffReview\":{\"version\":1,\"files\":{\"unstaged:src/app.ts\":{\"key\":\"unstaged:src/app.ts\",\"filePath\":\"src/app.ts\",\"scope\":\"unstaged\",\"lastSeenDiffIdentity\":\"identity-1\",\"reviewedAt\":1767225600000,\"reviewDiffIdentity\":\"identity-1\"}},\"updatedAt\":1767225600000,\"completedAt\":1767225600000}}}"
|
||||
},
|
||||
"8efeb57ebe41": {
|
||||
"name": "worktree.set#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -59,6 +64,17 @@
|
||||
"worktreeId": "workspace-1"
|
||||
}
|
||||
],
|
||||
"diffCommentsBase": [
|
||||
{
|
||||
"body": "needs a test",
|
||||
"createdAt": 0,
|
||||
"filePath": "src/app.ts",
|
||||
"id": "note-1",
|
||||
"lineNumber": 4,
|
||||
"side": "modified",
|
||||
"worktreeId": "workspace-1"
|
||||
}
|
||||
],
|
||||
"mobileDiffReview": {
|
||||
"completedAt": 1767225600000,
|
||||
"files": {
|
||||
@@ -113,11 +129,6 @@
|
||||
"message": "Workspace is locked",
|
||||
"isRpcDeliveryUnknown": false
|
||||
}
|
||||
},
|
||||
"bc47263d2197": {
|
||||
"name": "worktree.set#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.set\",\"params\":{\"worktree\":\"id:workspace-1\",\"diffComments\":[{\"side\":\"modified\",\"id\":\"note-1\",\"worktreeId\":\"workspace-1\",\"filePath\":\"src/app.ts\",\"lineNumber\":4,\"body\":\"needs a test\",\"createdAt\":0}],\"mobileDiffReview\":{\"version\":1,\"files\":{\"unstaged:src/app.ts\":{\"key\":\"unstaged:src/app.ts\",\"filePath\":\"src/app.ts\",\"scope\":\"unstaged\",\"lastSeenDiffIdentity\":\"identity-1\",\"reviewedAt\":1767225600000,\"reviewDiffIdentity\":\"identity-1\"}},\"updatedAt\":1767225600000,\"completedAt\":1767225600000}}}"
|
||||
}
|
||||
},
|
||||
"recording": {
|
||||
@@ -126,8 +137,8 @@
|
||||
{
|
||||
"id": "rolled-back",
|
||||
"observation": {
|
||||
"sender": ["ac014f21aa55"],
|
||||
"payloads": ["bc47263d2197"],
|
||||
"sender": ["8efeb57ebe41"],
|
||||
"payloads": ["81c9edeaad86"],
|
||||
"settlements": {
|
||||
"mark-reviewed": "b914541ed9b0"
|
||||
},
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
"$rpc": "null"
|
||||
}
|
||||
},
|
||||
"b596423bd056": {
|
||||
"52ec1b835753": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -33,7 +33,7 @@
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -66,7 +66,7 @@
|
||||
{
|
||||
"id": "unchanged",
|
||||
"observation": {
|
||||
"sender": ["b596423bd056"],
|
||||
"sender": ["52ec1b835753"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
|
||||
@@ -4,12 +4,7 @@
|
||||
"family": "session.diff-notes",
|
||||
"namedDeltas": [],
|
||||
"values": {
|
||||
"270b4ca66aa7": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.show\",\"params\":{\"worktree\":\"id:workspace-1\"}}"
|
||||
},
|
||||
"542d7b8c3e3f": {
|
||||
"194b8cd56a74": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 1,
|
||||
"args": [
|
||||
@@ -26,7 +21,7 @@
|
||||
{
|
||||
"name": "options",
|
||||
"value": {
|
||||
"$rpc": "absent"
|
||||
"failWhenDisconnected": true
|
||||
}
|
||||
}
|
||||
],
|
||||
@@ -54,6 +49,11 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"270b4ca66aa7": {
|
||||
"name": "worktree.show#1",
|
||||
"ordinal": 2,
|
||||
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"worktree.show\",\"params\":{\"worktree\":\"id:workspace-1\"}}"
|
||||
},
|
||||
"e28b1ad79121": {
|
||||
"busy": false,
|
||||
"diffComments": [
|
||||
@@ -108,7 +108,7 @@
|
||||
{
|
||||
"id": "loaded",
|
||||
"observation": {
|
||||
"sender": ["542d7b8c3e3f"],
|
||||
"sender": ["194b8cd56a74"],
|
||||
"payloads": ["270b4ca66aa7"],
|
||||
"settlements": {
|
||||
"mount": "eb79a9b3682a"
|
||||
|
||||
@@ -18476,6 +18476,17 @@
|
||||
"side": "modified"
|
||||
}
|
||||
],
|
||||
"diffCommentsBase": [
|
||||
{
|
||||
"body": "needs a test",
|
||||
"createdAt": 0,
|
||||
"filePath": "src/app.ts",
|
||||
"id": "note-1",
|
||||
"lineNumber": 4,
|
||||
"worktreeId": "workspace-1",
|
||||
"side": "modified"
|
||||
}
|
||||
],
|
||||
"mobileDiffReview": {
|
||||
"completedAt": 1767225600000,
|
||||
"files": {
|
||||
@@ -18542,6 +18553,17 @@
|
||||
"side": "modified"
|
||||
}
|
||||
],
|
||||
"diffCommentsBase": [
|
||||
{
|
||||
"body": "needs a test",
|
||||
"createdAt": 0,
|
||||
"filePath": "src/app.ts",
|
||||
"id": "note-1",
|
||||
"lineNumber": 4,
|
||||
"worktreeId": "workspace-1",
|
||||
"side": "modified"
|
||||
}
|
||||
],
|
||||
"mobileDiffReview": {
|
||||
"completedAt": 1767225600000,
|
||||
"files": {
|
||||
|
||||
@@ -0,0 +1,311 @@
|
||||
import { createElement, useEffect, useRef, useState } from 'react'
|
||||
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mergeDiffCommentMutation } from '../../../src/shared/diff-comment-mutation'
|
||||
import type { DiffComment } from '../../../src/shared/diff-comment-types'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import type { RpcResponse } from '../transport/types'
|
||||
import type { ReviewScreenState } from './mobile-diff-review-screen-model'
|
||||
import { useMobileDiffReviewCommentActions } from './use-mobile-diff-review-comment-actions'
|
||||
import { useMobileSessionDiffComments } from './use-mobile-session-diff-comments'
|
||||
|
||||
vi.mock('../platform/haptics', () => ({
|
||||
triggerSuccess: vi.fn(),
|
||||
triggerError: vi.fn(),
|
||||
triggerSelection: vi.fn()
|
||||
}))
|
||||
vi.mock('expo-clipboard', () => ({ setStringAsync: async () => true }))
|
||||
|
||||
function note(id: string): DiffComment {
|
||||
return {
|
||||
id,
|
||||
worktreeId: 'workspace-1',
|
||||
filePath: 'src/a.ts',
|
||||
lineNumber: 3,
|
||||
body: `note ${id}`,
|
||||
createdAt: 1,
|
||||
side: 'modified'
|
||||
}
|
||||
}
|
||||
|
||||
type SetParams = { diffComments: DiffComment[]; diffCommentsBase?: DiffComment[] }
|
||||
|
||||
/** A host holding the stored notes; `merges: false` is a host from before the base field. */
|
||||
function fakeHost(initial: DiffComment[], options: { merges: boolean }) {
|
||||
const host = { notes: initial, holdShow: false, holdSets: false, failNextSet: false }
|
||||
const heldShows: (() => void)[] = []
|
||||
const heldSets: (() => void)[] = []
|
||||
const reply = (result: unknown): RpcResponse => ({
|
||||
id: 'x',
|
||||
ok: true,
|
||||
result,
|
||||
_meta: { runtimeId: 'r' }
|
||||
})
|
||||
const sendRequest = vi.fn((method: string, params?: unknown): Promise<RpcResponse> => {
|
||||
if (method === 'worktree.show') {
|
||||
const snapshot = host.notes
|
||||
return new Promise((resolve) => {
|
||||
const answer = () => resolve(reply({ worktree: { diffComments: snapshot } }))
|
||||
if (host.holdShow) {
|
||||
heldShows.push(answer)
|
||||
} else {
|
||||
answer()
|
||||
}
|
||||
})
|
||||
}
|
||||
if (method === 'worktree.set') {
|
||||
if (host.failNextSet) {
|
||||
host.failNextSet = false
|
||||
return Promise.resolve({ id: 'x', ok: false, error: { code: 'x', message: 'Disk full' } })
|
||||
}
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the hooks under test send only this shape to worktree.set.
|
||||
const { diffComments, diffCommentsBase } = params as SetParams
|
||||
return new Promise((resolve) => {
|
||||
const apply = () => {
|
||||
host.notes =
|
||||
options.merges && diffCommentsBase
|
||||
? mergeDiffCommentMutation(diffCommentsBase, host.notes, diffComments)
|
||||
: diffComments
|
||||
resolve(reply({ worktree: { diffComments: host.notes } }))
|
||||
}
|
||||
if (host.holdSets) {
|
||||
heldSets.push(apply)
|
||||
} else {
|
||||
apply()
|
||||
}
|
||||
})
|
||||
}
|
||||
return new Promise<RpcResponse>(() => {})
|
||||
})
|
||||
const client: RpcClient = {
|
||||
sendRequest,
|
||||
subscribe: () => () => {},
|
||||
updateTerminalSubscriptionViewport: () => {},
|
||||
getState: () => 'connected',
|
||||
getReconnectAttempt: () => 0,
|
||||
getLastConnectedAt: () => null,
|
||||
onStateChange: () => () => {},
|
||||
notifyForeground: () => {},
|
||||
close: () => {}
|
||||
}
|
||||
return { host, client, heldShows, heldSets, sendRequest }
|
||||
}
|
||||
|
||||
type Notes = ReturnType<typeof useMobileSessionDiffComments>
|
||||
let renderer: ReactTestRenderer | undefined
|
||||
let notes: Notes | undefined
|
||||
let shown: DiffComment[] = []
|
||||
afterEach(() => {
|
||||
act(() => renderer?.unmount())
|
||||
renderer = undefined
|
||||
notes = undefined
|
||||
})
|
||||
|
||||
function hook(): Notes {
|
||||
if (!notes) {
|
||||
throw new Error('notes hook not mounted')
|
||||
}
|
||||
return notes
|
||||
}
|
||||
|
||||
async function mountNotes(client: RpcClient): Promise<void> {
|
||||
function Harness() {
|
||||
const [diffComments, setDiffComments] = useState<DiffComment[]>([])
|
||||
const diffCommentsRef = useRef(diffComments)
|
||||
const [diffCommentBusy, setDiffCommentBusy] = useState(false)
|
||||
// Mirrors the session screen, which syncs the ref after each render.
|
||||
useEffect(() => {
|
||||
diffCommentsRef.current = diffComments
|
||||
}, [diffComments])
|
||||
shown = diffComments
|
||||
const scope = {
|
||||
worktreeId: 'workspace-1',
|
||||
isFloatingWorkspaceRoute: false,
|
||||
client,
|
||||
connState: 'connected',
|
||||
setDiffComments,
|
||||
diffCommentsRef,
|
||||
diffCommentBusy,
|
||||
setDiffCommentBusy,
|
||||
setPendingDiffNotesDelivery: () => {},
|
||||
showToast: () => {}
|
||||
}
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the scope carries every member the notes hook reads; a missing one throws on use.
|
||||
notes = useMobileSessionDiffComments(scope as never)
|
||||
return null
|
||||
}
|
||||
await act(async () => {
|
||||
renderer = create(createElement(Harness))
|
||||
})
|
||||
}
|
||||
|
||||
const ids = (comments: readonly DiffComment[]) => comments.map((comment) => comment.id)
|
||||
|
||||
describe('mobile review notes saved against the host', () => {
|
||||
it('keeps a note added on desktop after the phone loaded when the phone adds and deletes', async () => {
|
||||
const { host, client } = fakeHost([note('a')], { merges: true })
|
||||
await mountNotes(client)
|
||||
host.notes = [...host.notes, note('desktop')]
|
||||
|
||||
await act(async () => {
|
||||
await hook().addDiffCommentForFile('src/a.ts', 9, 'from phone')
|
||||
})
|
||||
await act(async () => hook().deleteDiffCommentForFile('a'))
|
||||
|
||||
expect(ids(host.notes)).toEqual(['desktop', expect.stringMatching(/^mobile-/)])
|
||||
expect(ids(shown)).toEqual(ids(host.notes))
|
||||
})
|
||||
|
||||
it('saves both notes when two are added in the same frame', async () => {
|
||||
const { host, client } = fakeHost([note('a')], { merges: true })
|
||||
await mountNotes(client)
|
||||
|
||||
await act(async () => {
|
||||
await Promise.all([
|
||||
hook().addDiffCommentForFile('src/a.ts', 1, 'first'),
|
||||
hook().addDiffCommentForFile('src/a.ts', 2, 'second')
|
||||
])
|
||||
})
|
||||
|
||||
expect(host.notes.map((comment) => comment.body)).toEqual(['note a', 'first', 'second'])
|
||||
expect(ids(shown)).toEqual(ids(host.notes))
|
||||
})
|
||||
|
||||
it('does not let a slow first load replace a note saved after it', async () => {
|
||||
const { host, client, heldShows } = fakeHost([note('a')], { merges: true })
|
||||
host.holdShow = true
|
||||
await mountNotes(client)
|
||||
|
||||
await act(async () => {
|
||||
const adding = hook().addDiffCommentForFile('src/a.ts', 2, 'early')
|
||||
heldShows.forEach((answer) => answer())
|
||||
await adding
|
||||
})
|
||||
|
||||
expect(shown.map((comment) => comment.body)).toEqual(['note a', 'early'])
|
||||
})
|
||||
|
||||
it('still shows the loaded notes when a save made before the first load fails', async () => {
|
||||
const { host, client, heldShows } = fakeHost([note('a')], { merges: true })
|
||||
host.holdShow = true
|
||||
host.failNextSet = true
|
||||
await mountNotes(client)
|
||||
|
||||
await act(async () => {
|
||||
const adding = hook().addDiffCommentForFile('src/a.ts', 2, 'refused')
|
||||
heldShows.forEach((answer) => answer())
|
||||
await adding
|
||||
})
|
||||
|
||||
expect(ids(shown)).toEqual(['a'])
|
||||
})
|
||||
|
||||
it('keeps a saved note when a delivered-note cleanup is made while its save is in flight', async () => {
|
||||
const { host, client, heldSets } = fakeHost([note('a')], { merges: true })
|
||||
await mountNotes(client)
|
||||
host.holdSets = true
|
||||
|
||||
let adding: Promise<boolean> = Promise.resolve(false)
|
||||
let clearing: Promise<void> = Promise.resolve()
|
||||
await act(async () => {
|
||||
adding = hook().addDiffCommentForFile('src/a.ts', 2, 'kept')
|
||||
clearing = hook().clearDeliveredDiffComments(shown.filter((comment) => comment.id === 'a'))
|
||||
})
|
||||
await act(async () => {
|
||||
// Answer the newest held write first, the order a host can apply two in-flight writes in.
|
||||
for (let turn = 0; turn < 4; turn += 1) {
|
||||
heldSets.pop()?.()
|
||||
await new Promise((resolve) => setTimeout(resolve, 0))
|
||||
}
|
||||
await Promise.all([adding, clearing])
|
||||
})
|
||||
|
||||
expect(host.notes.map((comment) => comment.body)).toEqual(['kept'])
|
||||
expect(ids(shown)).toEqual(ids(host.notes))
|
||||
})
|
||||
|
||||
it('sends only its own change after a failed save, so a host without the merge keeps the note', async () => {
|
||||
const { host, client } = fakeHost([note('a'), note('b')], { merges: false })
|
||||
await mountNotes(client)
|
||||
host.failNextSet = true
|
||||
|
||||
await act(async () => {
|
||||
await Promise.all([
|
||||
hook().deleteDiffCommentForFile('a'),
|
||||
hook().addDiffCommentForFile('src/a.ts', 2, 'added')
|
||||
])
|
||||
})
|
||||
|
||||
expect(host.notes.map((comment) => comment.body)).toEqual(['note a', 'note b', 'added'])
|
||||
expect(ids(shown)).toEqual(ids(host.notes))
|
||||
})
|
||||
|
||||
it('keeps loaded notes on a host without the merge when a note is added before the first load', async () => {
|
||||
const { host, client, heldShows } = fakeHost([note('a')], { merges: false })
|
||||
host.holdShow = true
|
||||
await mountNotes(client)
|
||||
|
||||
await act(async () => {
|
||||
const adding = hook().addDiffCommentForFile('src/a.ts', 2, 'early')
|
||||
heldShows.forEach((answer) => answer())
|
||||
await adding
|
||||
})
|
||||
|
||||
expect(host.notes.map((comment) => comment.body)).toEqual(['note a', 'early'])
|
||||
})
|
||||
|
||||
it('still sends the full list, so a host without the merge replaces it as before', async () => {
|
||||
const { host, client, sendRequest } = fakeHost([note('a')], { merges: false })
|
||||
await mountNotes(client)
|
||||
|
||||
await act(async () => {
|
||||
await hook().addDiffCommentForFile('src/a.ts', 2, 'phone')
|
||||
})
|
||||
|
||||
const sent = sendRequest.mock.calls.find(([method]) => method === 'worktree.set')?.[1]
|
||||
expect(sent).toMatchObject({ diffComments: [note('a'), { body: 'phone' }] })
|
||||
expect(host.notes.map((comment) => comment.body)).toEqual(['note a', 'phone'])
|
||||
expect(shown).toEqual(host.notes)
|
||||
})
|
||||
})
|
||||
|
||||
describe('review screen notes saved against the host', () => {
|
||||
it('shows the host list after a save, so an edit to a note desktop deleted does not look saved', async () => {
|
||||
const { host, client } = fakeHost([note('a'), note('b')], { merges: true })
|
||||
let state: ReviewScreenState = {
|
||||
kind: 'ready',
|
||||
status: { entries: [] },
|
||||
branchCompare: null,
|
||||
comments: [note('a'), note('b')],
|
||||
reviewState: { version: 1, files: {} }
|
||||
}
|
||||
let actions: ReturnType<typeof useMobileDiffReviewCommentActions> | undefined
|
||||
function Harness() {
|
||||
const [screenState, setScreenState] = useState(state)
|
||||
state = screenState
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the save path reads only these members.
|
||||
actions = useMobileDiffReviewCommentActions({
|
||||
client,
|
||||
connState: 'connected',
|
||||
worktreeId: 'workspace-1',
|
||||
screenState,
|
||||
setScreenState,
|
||||
setActionError: () => {},
|
||||
sheets: {}
|
||||
} as never)
|
||||
return null
|
||||
}
|
||||
await act(async () => {
|
||||
renderer = create(createElement(Harness))
|
||||
})
|
||||
host.notes = [note('b')]
|
||||
|
||||
const edited = { ...note('a'), body: 'edited' }
|
||||
await act(async () => {
|
||||
await actions?.saveCommentsAndReviewState([edited, note('b')], { version: 1, files: {} })
|
||||
})
|
||||
|
||||
expect(ids(host.notes)).toEqual(['b'])
|
||||
expect(state.kind === 'ready' ? ids(state.comments) : []).toEqual(['b'])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,34 @@
|
||||
import { mergeDiffCommentMutation } from '../../../src/shared/diff-comment-mutation'
|
||||
import type { DiffComment } from '../../../src/shared/diff-comment-types'
|
||||
|
||||
type PendingSave = { base: DiffComment[]; next: DiffComment[] }
|
||||
|
||||
/** One workspace's notes: the host's last list, the phone's unanswered saves and their request queue. */
|
||||
export type ReviewNotesSync = {
|
||||
active: boolean
|
||||
host: DiffComment[]
|
||||
pending: PendingSave[]
|
||||
queue: Promise<unknown>
|
||||
}
|
||||
|
||||
export function createReviewNotesSync(): ReviewNotesSync {
|
||||
return { active: true, host: [], pending: [], queue: Promise.resolve() }
|
||||
}
|
||||
|
||||
// What the phone shows: the host's list with every unanswered phone save replayed on top.
|
||||
export function reviewNotesView(sync: ReviewNotesSync): DiffComment[] {
|
||||
return sync.pending.reduce(
|
||||
(list, save) => mergeDiffCommentMutation(save.base, list, save.next),
|
||||
sync.host
|
||||
)
|
||||
}
|
||||
|
||||
// Why: one request at a time, so each reply already holds this phone's earlier saves.
|
||||
export function enqueueReviewNotesRequest<T>(
|
||||
sync: ReviewNotesSync,
|
||||
run: () => Promise<T>
|
||||
): Promise<T> {
|
||||
const result = sync.queue.then(run)
|
||||
sync.queue = result.catch(() => undefined)
|
||||
return result
|
||||
}
|
||||
@@ -50,7 +50,7 @@ function launchReply(promptOutcome: 'handed-to-terminal' | 'not-delivered'): Rpc
|
||||
}
|
||||
}
|
||||
|
||||
/** Launch replies answered by hand; the notes reader never answers, so the seeded notes stand. */
|
||||
/** Launch replies answered by hand; the notes read answers with the seeded note. */
|
||||
function launchClient() {
|
||||
const replies: ((reply: RpcResponse) => void)[] = []
|
||||
const sendRequest = vi.fn(
|
||||
@@ -58,6 +58,13 @@ function launchClient() {
|
||||
new Promise<RpcResponse>((resolve) => {
|
||||
if (method === 'agent.launchReplay') {
|
||||
replies.push(resolve)
|
||||
} else if (method === 'worktree.show') {
|
||||
resolve({
|
||||
id: 'notes',
|
||||
ok: true,
|
||||
result: { worktree: { diffComments: [NOTE] } },
|
||||
_meta: { runtimeId: 'r' }
|
||||
})
|
||||
} else if (method === 'worktree.set') {
|
||||
resolve({ id: 'save', ok: true, result: {}, _meta: { runtimeId: 'r' } })
|
||||
}
|
||||
@@ -213,7 +220,11 @@ describe('sending review notes to a new agent session', () => {
|
||||
await act(async () => replies[0]?.(launchReply('handed-to-terminal')))
|
||||
expect(screen().sendingIds.size).toBe(0)
|
||||
expect(sendRequest.mock.calls.filter(([method]) => method === 'worktree.set')).toEqual([
|
||||
['worktree.set', { worktree: 'id:workspace-1', diffComments: [] }]
|
||||
[
|
||||
'worktree.set',
|
||||
{ worktree: 'id:workspace-1', diffComments: [], diffCommentsBase: [NOTE] },
|
||||
{}
|
||||
]
|
||||
])
|
||||
await act(async () => screen().send())
|
||||
expect(screen().pendingDelivery).toBeNull()
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { bindDeferredRpcOperation, defineRpcOperation } from '../transport/rpc-operation'
|
||||
import { rpcResultVariant } from '../transport/rpc-operation-result-reader'
|
||||
import { markdownTabDocumentSchema } from './session-read-reply-schema'
|
||||
import { markdownTabDocumentSchema, sessionWorktreeRecordSchema } from './session-read-reply-schema'
|
||||
import { terminalSendAcceptedSchema } from '../terminal/terminal-reply-schema'
|
||||
import {
|
||||
sessionCreatedTerminalTabSchema,
|
||||
@@ -92,14 +92,15 @@ export const sessionTabActivate = bindDeferredRpcOperation(
|
||||
})
|
||||
)
|
||||
|
||||
// Preserve host refusal text for optimistic rollback; review-note partial updates have a separate contract from source-control link writes.
|
||||
// Preserve host refusal text for optimistic rollback; the reply's record carries the host's merged notes.
|
||||
export const sessionWorktreeNotesWrite = bindDeferredRpcOperation(
|
||||
defineRpcOperation({
|
||||
name: 'worktree.set-review-notes',
|
||||
method: 'worktree.set',
|
||||
acceptance: 'require-result-or-throw-message',
|
||||
barrier: 'after-caller-barrier',
|
||||
read: rpcResultVariant('worktree-notes-written', sessionWriteUnreadReplySchema)
|
||||
// Acceptance stays as before; an unreadable record only means no merged notes to show.
|
||||
read: rpcResultVariant('worktree-notes-written', sessionWorktreeRecordSchema.catch(undefined))
|
||||
})
|
||||
)
|
||||
|
||||
|
||||
@@ -5,7 +5,11 @@ import type { ConnectionState } from '../transport/types'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { interpretOrThrowRefusalMessage } from '../transport/rpc-refusal-message'
|
||||
import { sessionWorktreeNotesWrite } from './mobile-session-write-operations'
|
||||
import { addMobileDiffComment, removeMobileDiffComments } from './mobile-diff-comments'
|
||||
import {
|
||||
addMobileDiffComment,
|
||||
normalizeMobileDiffComments,
|
||||
removeMobileDiffComments
|
||||
} from './mobile-diff-comments'
|
||||
import { updateMobileDiffComment } from './mobile-diff-comment-edit'
|
||||
import {
|
||||
clearMobileDiffReviewFileReviewed,
|
||||
@@ -64,19 +68,24 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
|
||||
const { openSheet, openSheetWhenIdle, closeSheet } = sheets
|
||||
|
||||
const persistMetadata = useCallback(
|
||||
async (comments: readonly DiffComment[], reviewState: MobileDiffReviewState) => {
|
||||
async (
|
||||
comments: readonly DiffComment[],
|
||||
base: readonly DiffComment[],
|
||||
reviewState: MobileDiffReviewState
|
||||
) => {
|
||||
if (!client || connState !== 'connected') {
|
||||
throw new Error('Waiting for desktop...')
|
||||
}
|
||||
const response = await sessionWorktreeNotesWrite.request(client, {
|
||||
worktree: `id:${worktreeId}`,
|
||||
diffComments: [...comments],
|
||||
diffCommentsBase: [...base],
|
||||
mobileDiffReview: reviewState
|
||||
})
|
||||
interpretOrThrowRefusalMessage(
|
||||
return interpretOrThrowRefusalMessage(
|
||||
() => sessionWorktreeNotesWrite.interpret(response),
|
||||
'Failed to save review state'
|
||||
)
|
||||
)?.diffComments
|
||||
},
|
||||
[client, connState, worktreeId]
|
||||
)
|
||||
@@ -93,7 +102,19 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
|
||||
const previous = screenState
|
||||
updateReadyState((state) => ({ ...state, comments, reviewState }))
|
||||
try {
|
||||
await persistMetadata(comments, reviewState)
|
||||
const saved = await persistMetadata(
|
||||
comments,
|
||||
previous.kind === 'ready' ? previous.comments : comments,
|
||||
reviewState
|
||||
)
|
||||
if (saved !== undefined) {
|
||||
// Why: the host kept notes other devices changed; show its list unless a later save moved on.
|
||||
updateReadyState((state) =>
|
||||
state.comments === comments
|
||||
? { ...state, comments: normalizeMobileDiffComments(saved, worktreeId) }
|
||||
: state
|
||||
)
|
||||
}
|
||||
triggerSuccess()
|
||||
} catch (err) {
|
||||
if (previous.kind === 'ready') {
|
||||
@@ -104,7 +125,7 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
|
||||
throw err
|
||||
}
|
||||
},
|
||||
[persistMetadata, screenState, setActionError, setScreenState, updateReadyState]
|
||||
[persistMetadata, screenState, setActionError, setScreenState, updateReadyState, worktreeId]
|
||||
)
|
||||
|
||||
const openComposer = useCallback(
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { useEffect, useCallback, useRef, useState } from 'react'
|
||||
import { useEffect, useCallback, useMemo, useRef, useState } from 'react'
|
||||
import { formatDiffComments } from '../../../src/shared/diff-comments-format'
|
||||
import { useClipboardWriter } from '../platform/clipboard'
|
||||
import { interpretOrThrowRefusalMessage } from '../transport/rpc-refusal-message'
|
||||
import type { SendRequestOptions } from '../transport/unvalidated-rpc-request-port'
|
||||
import { sessionWorktreeRecordRead } from './mobile-session-read-operations'
|
||||
import { sessionWorktreeNotesWrite } from './mobile-session-write-operations'
|
||||
import { triggerSelection, triggerSuccess, triggerError } from '../platform/haptics'
|
||||
@@ -12,10 +13,22 @@ import {
|
||||
removeMobileDiffComments,
|
||||
sendableMobileDiffComments
|
||||
} from './mobile-diff-comments'
|
||||
import { mergeDiffCommentMutation } from '../../../src/shared/diff-comment-mutation'
|
||||
import {
|
||||
createReviewNotesSync,
|
||||
enqueueReviewNotesRequest,
|
||||
reviewNotesView,
|
||||
type ReviewNotesSync
|
||||
} from './mobile-review-notes-sync'
|
||||
import type { DiffComment } from '../../../src/shared/diff-comment-types'
|
||||
import type { DiffNotesDelivery } from './mobile-session-route-types'
|
||||
import type { MobileSessionDocumentReadersModel } from './use-mobile-session-document-readers'
|
||||
|
||||
// Why: a queued request must not wait untimed for a reconnect and stall the notes behind it.
|
||||
const NOTES_REQUEST_OPTIONS: SendRequestOptions = { failWhenDisconnected: true }
|
||||
// Why: delivered notes must still clear after a reconnect, or they could be sent again.
|
||||
const DELIVERED_CLEANUP_REQUEST_OPTIONS: SendRequestOptions = {}
|
||||
|
||||
export function useMobileSessionDiffComments(scope: MobileSessionDocumentReadersModel) {
|
||||
const {
|
||||
worktreeId,
|
||||
@@ -35,37 +48,94 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders
|
||||
const [sendingDiffCommentIds, setSendingDiffCommentIds] = useState<ReadonlySet<string>>(
|
||||
sendingDiffCommentIdsRef.current
|
||||
)
|
||||
const loadDiffComments = useCallback(async (): Promise<void> => {
|
||||
if (!client || connState !== 'connected' || !worktreeId || isFloatingWorkspaceRoute) {
|
||||
setDiffComments([])
|
||||
return
|
||||
}
|
||||
const response = sessionWorktreeRecordRead.interpret(
|
||||
await sessionWorktreeRecordRead.request(client, { worktree: `id:${worktreeId}` })
|
||||
)
|
||||
if (!response.accepted) {
|
||||
return
|
||||
}
|
||||
setDiffComments(normalizeMobileDiffComments(response.value?.diffComments, worktreeId))
|
||||
}, [client, connState, worktreeId, isFloatingWorkspaceRoute])
|
||||
|
||||
const persistDiffComments = useCallback(
|
||||
async (comments: readonly DiffComment[]): Promise<void> => {
|
||||
if (!client || connState !== 'connected') {
|
||||
throw new Error('Waiting for desktop...')
|
||||
// Why: a reply or failure for another workspace or connection must not touch these notes.
|
||||
const notesSync = useMemo(() => createReviewNotesSync(), [client, worktreeId])
|
||||
const showDiffComments = useCallback(
|
||||
(sync: ReviewNotesSync): void => {
|
||||
if (!sync.active) {
|
||||
return
|
||||
}
|
||||
const response = await sessionWorktreeNotesWrite.request(client, {
|
||||
worktree: `id:${worktreeId}`,
|
||||
diffComments: [...comments]
|
||||
})
|
||||
interpretOrThrowRefusalMessage(
|
||||
() => sessionWorktreeNotesWrite.interpret(response),
|
||||
'Failed to save review notes'
|
||||
)
|
||||
const comments = reviewNotesView(sync)
|
||||
// Why: a second tap in the same frame must build on this list, not the last rendered one.
|
||||
diffCommentsRef.current = comments
|
||||
setDiffComments(comments)
|
||||
},
|
||||
[client, connState, worktreeId]
|
||||
[diffCommentsRef, setDiffComments]
|
||||
)
|
||||
|
||||
const loadDiffComments = useCallback((): Promise<void> => {
|
||||
const sync = notesSync
|
||||
return enqueueReviewNotesRequest(sync, async () => {
|
||||
if (!client || connState !== 'connected' || !worktreeId || isFloatingWorkspaceRoute) {
|
||||
sync.host = []
|
||||
showDiffComments(sync)
|
||||
return
|
||||
}
|
||||
const response = sessionWorktreeRecordRead.interpret(
|
||||
await sessionWorktreeRecordRead.request(
|
||||
client,
|
||||
{ worktree: `id:${worktreeId}` },
|
||||
NOTES_REQUEST_OPTIONS
|
||||
)
|
||||
)
|
||||
if (response.accepted) {
|
||||
sync.host = normalizeMobileDiffComments(response.value?.diffComments, worktreeId)
|
||||
showDiffComments(sync)
|
||||
}
|
||||
})
|
||||
}, [client, connState, worktreeId, isFloatingWorkspaceRoute, notesSync, showDiffComments])
|
||||
|
||||
// Shows `next` at once, then the host's merged list; on failure drops only this change.
|
||||
const saveDiffComments = useCallback(
|
||||
async (next: DiffComment[], options: SendRequestOptions = NOTES_REQUEST_OPTIONS) => {
|
||||
const sync = notesSync
|
||||
const save = { base: diffCommentsRef.current, next }
|
||||
sync.pending = [...sync.pending, save]
|
||||
showDiffComments(sync)
|
||||
try {
|
||||
await enqueueReviewNotesRequest(sync, async () => {
|
||||
if (!client || connState !== 'connected') {
|
||||
throw new Error('Waiting for desktop...')
|
||||
}
|
||||
// Why: send the host's list plus only this change; an older host stores the list whole.
|
||||
const base = sync.host
|
||||
const rebased = mergeDiffCommentMutation(save.base, base, save.next)
|
||||
const response = await sessionWorktreeNotesWrite.request(
|
||||
client,
|
||||
{
|
||||
worktree: `id:${worktreeId}`,
|
||||
diffComments: [...rebased],
|
||||
diffCommentsBase: [...base]
|
||||
},
|
||||
options
|
||||
)
|
||||
const saved = interpretOrThrowRefusalMessage(
|
||||
() => sessionWorktreeNotesWrite.interpret(response),
|
||||
'Failed to save review notes'
|
||||
)
|
||||
sync.pending = sync.pending.filter((pending) => pending !== save)
|
||||
sync.host =
|
||||
saved?.diffComments !== undefined
|
||||
? normalizeMobileDiffComments(saved.diffComments, worktreeId)
|
||||
: rebased
|
||||
showDiffComments(sync)
|
||||
})
|
||||
} catch (err) {
|
||||
sync.pending = sync.pending.filter((pending) => pending !== save)
|
||||
showDiffComments(sync)
|
||||
throw err
|
||||
}
|
||||
},
|
||||
[client, connState, worktreeId, diffCommentsRef, notesSync, showDiffComments]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
notesSync.active = true
|
||||
return () => {
|
||||
notesSync.active = false
|
||||
}
|
||||
}, [notesSync])
|
||||
|
||||
useEffect(() => {
|
||||
// Catch at the fire-and-forget mount caller; the recording adapter still needs the loader’s rejection.
|
||||
void loadDiffComments().catch(() => undefined)
|
||||
@@ -88,16 +158,13 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders
|
||||
if (!result.comment) {
|
||||
return false
|
||||
}
|
||||
const previous = diffCommentsRef.current
|
||||
setDiffCommentBusy(true)
|
||||
setDiffComments(result.comments)
|
||||
try {
|
||||
await persistDiffComments(result.comments)
|
||||
await saveDiffComments(result.comments)
|
||||
triggerSuccess()
|
||||
showToast('Note added')
|
||||
return true
|
||||
} catch (err) {
|
||||
setDiffComments(previous)
|
||||
triggerError()
|
||||
showToast(err instanceof Error ? err.message : 'Failed to save note', 1600)
|
||||
return false
|
||||
@@ -105,7 +172,7 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders
|
||||
setDiffCommentBusy(false)
|
||||
}
|
||||
},
|
||||
[diffCommentBusy, persistDiffComments, showToast, worktreeId]
|
||||
[diffCommentBusy, saveDiffComments, showToast, worktreeId]
|
||||
)
|
||||
|
||||
const deleteDiffCommentForFile = useCallback(
|
||||
@@ -119,19 +186,17 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders
|
||||
return
|
||||
}
|
||||
setDiffCommentBusy(true)
|
||||
setDiffComments(next)
|
||||
try {
|
||||
await persistDiffComments(next)
|
||||
await saveDiffComments(next)
|
||||
triggerSelection()
|
||||
} catch (err) {
|
||||
setDiffComments(previous)
|
||||
triggerError()
|
||||
showToast(err instanceof Error ? err.message : 'Failed to delete note', 1600)
|
||||
} finally {
|
||||
setDiffCommentBusy(false)
|
||||
}
|
||||
},
|
||||
[diffCommentBusy, persistDiffComments, showToast]
|
||||
[diffCommentBusy, saveDiffComments, showToast]
|
||||
)
|
||||
|
||||
const copyDiffCommentsToClipboard = useCallback(async (): Promise<void> => {
|
||||
@@ -195,20 +260,18 @@ export function useMobileSessionDiffComments(scope: MobileSessionDocumentReaders
|
||||
return
|
||||
}
|
||||
setDiffCommentBusy(true)
|
||||
setDiffComments(next)
|
||||
try {
|
||||
await persistDiffComments(next)
|
||||
await saveDiffComments(next, DELIVERED_CLEANUP_REQUEST_OPTIONS)
|
||||
} catch {
|
||||
setDiffComments(previous)
|
||||
// saveDiffComments already restored the delivered notes.
|
||||
} finally {
|
||||
setDiffCommentBusy(false)
|
||||
}
|
||||
},
|
||||
[persistDiffComments]
|
||||
[saveDiffComments]
|
||||
)
|
||||
return {
|
||||
loadDiffComments,
|
||||
persistDiffComments,
|
||||
addDiffCommentForFile,
|
||||
deleteDiffCommentForFile,
|
||||
copyDiffCommentsToClipboard,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { WorkspaceAttachmentMutation } from '../../../shared/workspace-attachment-mutation'
|
||||
import type { DiffCommentBaseMutation } from '../../../shared/diff-comment-mutation'
|
||||
import type { WorkspaceKey } from '../../../shared/folder-workspace-types'
|
||||
import type { WorkspaceLineage, WorktreeLineage } from '../../../shared/worktree/lineage-types'
|
||||
import type { WorktreeMeta } from '../../../shared/worktree/meta-types'
|
||||
@@ -97,7 +98,7 @@ export class MetadataLineageOperations {
|
||||
|
||||
setWorktreeMeta(
|
||||
target: HostWorktreeTarget,
|
||||
meta: Partial<WorktreeMeta> & WorkspaceAttachmentMutation
|
||||
meta: Partial<WorktreeMeta> & WorkspaceAttachmentMutation & DiffCommentBaseMutation
|
||||
): WorktreeMeta {
|
||||
return setWorktreeMetaForHostOperation(
|
||||
this[metadataLineageOperationsContext].runtime,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { WorkspaceAttachmentMutation } from '../../../shared/workspace-attachment-mutation'
|
||||
import type { DiffCommentBaseMutation } from '../../../shared/diff-comment-mutation'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import type { WorktreeMeta } from '../../../shared/worktree/meta-types'
|
||||
import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../shared/execution-host'
|
||||
@@ -227,7 +228,7 @@ export function setWorktreeMetaForHost(
|
||||
scheduling: WriteSchedulingOperations,
|
||||
worktreeId: string,
|
||||
requestedHostId: ExecutionHostId,
|
||||
meta: Partial<WorktreeMeta> & WorkspaceAttachmentMutation
|
||||
meta: Partial<WorktreeMeta> & WorkspaceAttachmentMutation & DiffCommentBaseMutation
|
||||
): WorktreeMeta {
|
||||
const state = runtime.state
|
||||
const executionHostId = worktreeMetaHostFor(worktreeId, requestedHostId)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { DiffComment } from '../../../shared/diff-comment-types'
|
||||
import type { WorktreeMeta } from '../../../shared/worktree/meta-types'
|
||||
import { mergeWorktreeMetaForWrite } from './worktree-meta-write-normalization'
|
||||
import { CLEARED_HOSTED_REVIEW_LINK_UPDATES } from '../../../shared/worktree/hosted-review-links'
|
||||
@@ -17,7 +18,37 @@ const existingMeta: WorktreeMeta = {
|
||||
lastActivityAt: 0
|
||||
}
|
||||
|
||||
function note(id: string): DiffComment {
|
||||
return {
|
||||
id,
|
||||
worktreeId: 'wt',
|
||||
filePath: 'a.ts',
|
||||
lineNumber: 1,
|
||||
body: id,
|
||||
createdAt: 1,
|
||||
side: 'modified'
|
||||
}
|
||||
}
|
||||
|
||||
describe('worktree metadata write normalization', () => {
|
||||
it('applies a stale review-note writer’s own add and delete without dropping notes added elsewhere', () => {
|
||||
const existing = { ...existingMeta, diffComments: [note('a'), note('desktop')] }
|
||||
const updated = mergeWorktreeMetaForWrite(existing, {
|
||||
diffCommentsBase: [note('a')],
|
||||
diffComments: [note('phone')]
|
||||
})
|
||||
|
||||
expect(updated.diffComments).toEqual([note('desktop'), note('phone')])
|
||||
expect(updated).not.toHaveProperty('diffCommentsBase')
|
||||
})
|
||||
|
||||
it('replaces review notes outright for a writer that sends no base', () => {
|
||||
const existing = { ...existingMeta, diffComments: [note('a'), note('desktop')] }
|
||||
const updated = mergeWorktreeMetaForWrite(existing, { diffComments: [note('phone')] })
|
||||
|
||||
expect(updated.diffComments).toEqual([note('phone')])
|
||||
})
|
||||
|
||||
it('retains all five PRs while each successive branch clears its active review', () => {
|
||||
let meta = existingMeta
|
||||
const numbers = [32201, 32206, 32211, 32216, 32218]
|
||||
|
||||
@@ -9,6 +9,10 @@ import { WORKTREE_META_PERSISTED_DEFAULTS } from '../../../shared/worktree/meta-
|
||||
import { normalizeGitHubPRSuppressionUpdate } from '../../../shared/worktree/github-pr-suppression'
|
||||
import type { WorkspaceAttachmentMutation } from '../../../shared/workspace-attachment-mutation'
|
||||
import { normalizeWorkspaceAttachmentUpdate } from '../../../shared/workspace-attachments'
|
||||
import {
|
||||
mergeDiffCommentMutation,
|
||||
type DiffCommentBaseMutation
|
||||
} from '../../../shared/diff-comment-mutation'
|
||||
|
||||
type WorktreeMetaIdentity = {
|
||||
instanceId: string
|
||||
@@ -31,10 +35,18 @@ function createDefaultWorktreeMeta(): WorktreeMeta {
|
||||
|
||||
export function mergeWorktreeMetaForWrite(
|
||||
existing: WorktreeMeta | undefined,
|
||||
updates: Partial<WorktreeMeta> & WorkspaceAttachmentMutation,
|
||||
updates: Partial<WorktreeMeta> & WorkspaceAttachmentMutation & DiffCommentBaseMutation,
|
||||
identity?: WorktreeMetaIdentity
|
||||
): WorktreeMeta {
|
||||
const normalizedUpdates = normalizeWorkspaceAttachmentUpdate(existing, updates)
|
||||
const { diffCommentsBase, ...metaUpdates } = updates
|
||||
if (diffCommentsBase !== undefined && metaUpdates.diffComments !== undefined) {
|
||||
metaUpdates.diffComments = mergeDiffCommentMutation(
|
||||
diffCommentsBase,
|
||||
existing?.diffComments ?? [],
|
||||
metaUpdates.diffComments
|
||||
)
|
||||
}
|
||||
const normalizedUpdates = normalizeWorkspaceAttachmentUpdate(existing, metaUpdates)
|
||||
const updated = {
|
||||
...(existing ?? createDefaultWorktreeMeta()),
|
||||
...normalizeGitHubPRSuppressionUpdate(normalizedUpdates),
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import '../unused-default-rpc-methods.test-fixture'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { RpcDispatcher } from '../dispatcher'
|
||||
import type { OrcaRuntimeService } from '../../orca-runtime'
|
||||
import { WORKTREE_METHODS } from './worktree'
|
||||
|
||||
describe('worktree.set', () => {
|
||||
it('forwards the review-note base through worktree.set', async () => {
|
||||
const runtime = {
|
||||
getRuntimeId: () => 'test-runtime',
|
||||
updateManagedWorktreeMeta: vi.fn().mockResolvedValue({ id: 'wt-1' })
|
||||
}
|
||||
const dispatcher = new RpcDispatcher({
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: worktree.set reads only these two runtime members.
|
||||
runtime: runtime as unknown as OrcaRuntimeService,
|
||||
methods: WORKTREE_METHODS
|
||||
})
|
||||
const comment = {
|
||||
id: 'n1',
|
||||
worktreeId: 'wt-1',
|
||||
filePath: 'a.ts',
|
||||
lineNumber: 1,
|
||||
body: 'b',
|
||||
createdAt: 1,
|
||||
side: 'modified'
|
||||
}
|
||||
|
||||
await dispatcher.dispatch({
|
||||
id: 'req-1',
|
||||
authToken: 'tok',
|
||||
method: 'worktree.set',
|
||||
params: { worktree: 'id:wt-1', diffComments: [comment], diffCommentsBase: [] }
|
||||
})
|
||||
|
||||
expect(runtime.updateManagedWorktreeMeta).toHaveBeenCalledWith(
|
||||
'id:wt-1',
|
||||
expect.objectContaining({ diffComments: [comment], diffCommentsBase: [] })
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,67 @@
|
||||
import { WorktreeSet } from '../../../../shared/rpc-contract/worktree-params'
|
||||
import { displayNameUpdatePinsLabel } from '../../../../shared/worktree/display-name-provenance'
|
||||
import { getWorkspaceStatusWarning } from '../../../../shared/workspace-status-warning'
|
||||
import { defineMethod } from '../core'
|
||||
|
||||
export const WORKTREE_SET_METHOD = defineMethod({
|
||||
name: 'worktree.set',
|
||||
permission: 'workspace',
|
||||
params: WorktreeSet,
|
||||
handler: async (params, { runtime }) => {
|
||||
const worktree = await runtime.updateManagedWorktreeMeta(params.worktree, {
|
||||
displayName: params.displayName,
|
||||
...(params.displayName !== undefined
|
||||
? { displayNameIsPinned: displayNameUpdatePinsLabel(params.displayName) }
|
||||
: {}),
|
||||
linkedIssue: params.linkedIssue,
|
||||
linkedPR: params.linkedPR,
|
||||
suppressedGitHubPR: params.suppressedGitHubPR,
|
||||
linkedLinearIssue: params.linkedLinearIssue,
|
||||
linkedLinearIssueWorkspaceId: params.linkedLinearIssueWorkspaceId,
|
||||
linkedLinearIssueOrganizationUrlKey: params.linkedLinearIssueOrganizationUrlKey,
|
||||
linkedGitLabMR: params.linkedGitLabMR,
|
||||
linkedGitLabIssue: params.linkedGitLabIssue,
|
||||
linkedBitbucketPR: params.linkedBitbucketPR,
|
||||
linkedAzureDevOpsPR: params.linkedAzureDevOpsPR,
|
||||
linkedGiteaPR: params.linkedGiteaPR,
|
||||
linkedWorkItem: params.linkedWorkItem,
|
||||
linkedItems: params.linkedItems,
|
||||
...(params.linkedItemsBase !== undefined ? { linkedItemsBase: params.linkedItemsBase } : {}),
|
||||
...(params.linkedItemsSelectionChanged !== undefined
|
||||
? { linkedItemsSelectionChanged: params.linkedItemsSelectionChanged }
|
||||
: {}),
|
||||
linkedTaskSourceContext: params.linkedTaskSourceContext,
|
||||
comment: params.comment,
|
||||
isArchived: params.isArchived,
|
||||
isUnread: params.isUnread,
|
||||
isPinned: params.isPinned,
|
||||
sortOrder: params.sortOrder,
|
||||
manualOrder: params.manualOrder,
|
||||
lastActivityAt: params.lastActivityAt,
|
||||
createdAt: params.createdAt,
|
||||
sparseDirectories: params.sparseDirectories,
|
||||
sparseBaseRef: params.sparseBaseRef,
|
||||
sparsePresetId: params.sparsePresetId,
|
||||
baseRef: params.baseRef,
|
||||
workspaceStatus: params.workspaceStatus,
|
||||
pushTarget: params.pushTarget,
|
||||
diffComments: params.diffComments,
|
||||
...(params.diffCommentsBase !== undefined
|
||||
? { diffCommentsBase: params.diffCommentsBase }
|
||||
: {}),
|
||||
mobileDiffReview: params.mobileDiffReview,
|
||||
lineage:
|
||||
params.parentWorktree || params.noParent === true
|
||||
? {
|
||||
parentWorktree: params.parentWorktree,
|
||||
noParent: params.noParent === true
|
||||
}
|
||||
: undefined
|
||||
})
|
||||
const warning = getWorkspaceStatusWarning(
|
||||
params.workspaceStatus,
|
||||
() => runtime.getUIState().workspaceStatuses
|
||||
)
|
||||
return { worktree, ...(warning ? { warnings: [warning] } : {}) }
|
||||
}
|
||||
})
|
||||
@@ -7,8 +7,6 @@ import { getLocalWorktreeCatalogVersion } from '../../../local-worktree-scan-gen
|
||||
import { getExplicitWorktreeIdSelector } from '../../runtime-worktree-selection'
|
||||
import { splitWorktreeId } from '../../../../shared/worktree/id'
|
||||
import { buildCliWorkspaceProvenance } from '../../../../shared/cli-workspace-provenance'
|
||||
import { displayNameUpdatePinsLabel } from '../../../../shared/worktree/display-name-provenance'
|
||||
import { getWorkspaceStatusWarning } from '../../../../shared/workspace-status-warning'
|
||||
import { createWorktreeWithStartupAgent } from '../../../agent-launch/startup-agent-worktree-create'
|
||||
import { defineMethod } from '../core'
|
||||
import { buildManagedWorktreeCreateArgs, worktreeCreateRepoSelector } from './worktree-create-args'
|
||||
@@ -26,7 +24,6 @@ import {
|
||||
WorktreeResolveMrBase,
|
||||
WorktreeResolvePrBase,
|
||||
WorktreeSelector,
|
||||
WorktreeSet,
|
||||
WorktreeSortOrder,
|
||||
WorktreeTeardownMissingTerminalsParams
|
||||
} from '../../../../shared/rpc-contract/worktree-params'
|
||||
@@ -35,6 +32,7 @@ import { WORKTREE_PROVISIONED_ROOT_METHOD } from './worktree-provisioned-root-me
|
||||
import { readsWorktreeRemovalMarker } from '../worktree-removal-marker-projection'
|
||||
import { queueWorkspaceDelete } from '../../workspaces/workspace-lifecycle-commands'
|
||||
import { WORKTREE_SLEEP_METHOD } from './worktree-sleep-method'
|
||||
import { WORKTREE_SET_METHOD } from './worktree-set-method'
|
||||
|
||||
export const WORKTREE_METHODS = [
|
||||
...WORKTREE_CATALOG_METHODS,
|
||||
@@ -142,67 +140,7 @@ export const WORKTREE_METHODS = [
|
||||
return null
|
||||
}
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'worktree.set',
|
||||
permission: 'workspace',
|
||||
params: WorktreeSet,
|
||||
handler: async (params, { runtime }) => {
|
||||
const worktree = await runtime.updateManagedWorktreeMeta(params.worktree, {
|
||||
displayName: params.displayName,
|
||||
...(params.displayName !== undefined
|
||||
? { displayNameIsPinned: displayNameUpdatePinsLabel(params.displayName) }
|
||||
: {}),
|
||||
linkedIssue: params.linkedIssue,
|
||||
linkedPR: params.linkedPR,
|
||||
suppressedGitHubPR: params.suppressedGitHubPR,
|
||||
linkedLinearIssue: params.linkedLinearIssue,
|
||||
linkedLinearIssueWorkspaceId: params.linkedLinearIssueWorkspaceId,
|
||||
linkedLinearIssueOrganizationUrlKey: params.linkedLinearIssueOrganizationUrlKey,
|
||||
linkedGitLabMR: params.linkedGitLabMR,
|
||||
linkedGitLabIssue: params.linkedGitLabIssue,
|
||||
linkedBitbucketPR: params.linkedBitbucketPR,
|
||||
linkedAzureDevOpsPR: params.linkedAzureDevOpsPR,
|
||||
linkedGiteaPR: params.linkedGiteaPR,
|
||||
linkedWorkItem: params.linkedWorkItem,
|
||||
linkedItems: params.linkedItems,
|
||||
...(params.linkedItemsBase !== undefined
|
||||
? { linkedItemsBase: params.linkedItemsBase }
|
||||
: {}),
|
||||
...(params.linkedItemsSelectionChanged !== undefined
|
||||
? { linkedItemsSelectionChanged: params.linkedItemsSelectionChanged }
|
||||
: {}),
|
||||
linkedTaskSourceContext: params.linkedTaskSourceContext,
|
||||
comment: params.comment,
|
||||
isArchived: params.isArchived,
|
||||
isUnread: params.isUnread,
|
||||
isPinned: params.isPinned,
|
||||
sortOrder: params.sortOrder,
|
||||
manualOrder: params.manualOrder,
|
||||
lastActivityAt: params.lastActivityAt,
|
||||
createdAt: params.createdAt,
|
||||
sparseDirectories: params.sparseDirectories,
|
||||
sparseBaseRef: params.sparseBaseRef,
|
||||
sparsePresetId: params.sparsePresetId,
|
||||
baseRef: params.baseRef,
|
||||
workspaceStatus: params.workspaceStatus,
|
||||
pushTarget: params.pushTarget,
|
||||
diffComments: params.diffComments,
|
||||
mobileDiffReview: params.mobileDiffReview,
|
||||
lineage:
|
||||
params.parentWorktree || params.noParent === true
|
||||
? {
|
||||
parentWorktree: params.parentWorktree,
|
||||
noParent: params.noParent === true
|
||||
}
|
||||
: undefined
|
||||
})
|
||||
const warning = getWorkspaceStatusWarning(
|
||||
params.workspaceStatus,
|
||||
() => runtime.getUIState().workspaceStatuses
|
||||
)
|
||||
return { worktree, ...(warning ? { warnings: [warning] } : {}) }
|
||||
}
|
||||
}),
|
||||
WORKTREE_SET_METHOD,
|
||||
defineMethod({
|
||||
name: 'worktree.persistSortOrder',
|
||||
permission: 'workspace',
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import type { DiffComment } from './diff-comment-types'
|
||||
|
||||
/** Mutation-only snapshot; never persisted with workspace metadata. */
|
||||
export type DiffCommentBaseMutation = {
|
||||
diffCommentsBase?: DiffComment[]
|
||||
}
|
||||
|
||||
// Why: a writer's list can predate another device's edits, so apply only what it changed since its base.
|
||||
export function mergeDiffCommentMutation(
|
||||
base: readonly DiffComment[],
|
||||
current: readonly DiffComment[],
|
||||
requested: readonly DiffComment[]
|
||||
): DiffComment[] {
|
||||
const baseById = new Map(base.map((comment) => [comment.id, comment]))
|
||||
const requestedById = new Map(requested.map((comment) => [comment.id, comment]))
|
||||
const retained = current.flatMap((comment) => {
|
||||
const before = baseById.get(comment.id)
|
||||
if (!before) {
|
||||
return [comment]
|
||||
}
|
||||
const after = requestedById.get(comment.id)
|
||||
if (!after) {
|
||||
return []
|
||||
}
|
||||
return [JSON.stringify(before) === JSON.stringify(after) ? comment : after]
|
||||
})
|
||||
const currentIds = new Set(current.map((comment) => comment.id))
|
||||
const added = requested.filter(
|
||||
(comment) => !baseById.has(comment.id) && !currentIds.has(comment.id)
|
||||
)
|
||||
return [...retained, ...added]
|
||||
}
|
||||
@@ -161,6 +161,8 @@ export const WorktreeSet = WorktreeSelector.extend({
|
||||
.optional(),
|
||||
// Older clients may send unsupported review shapes alongside unrelated metadata edits.
|
||||
diffComments: z.array(DiffCommentSchema).optional().catch(undefined),
|
||||
// The list the writer started from; the host applies only its changes so other devices' notes survive.
|
||||
diffCommentsBase: z.array(DiffCommentSchema).optional().catch(undefined),
|
||||
mobileDiffReview: MobileDiffReviewSchema.optional().catch(undefined),
|
||||
parentWorktree: OptionalString,
|
||||
noParent: OptionalBoolean
|
||||
|
||||
Reference in New Issue
Block a user