diff --git a/mobile/rpc-foundation/goldens/matrix-session.diff-notes-worktree.show-1.json b/mobile/rpc-foundation/goldens/matrix-session.diff-notes-worktree.show-1.json index 55973e71815..62160c72758 100644 --- a/mobile/rpc-foundation/goldens/matrix-session.diff-notes-worktree.show-1.json +++ b/mobile/rpc-foundation/goldens/matrix-session.diff-notes-worktree.show-1.json @@ -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" diff --git a/mobile/rpc-foundation/goldens/matrix-session.diff-review-actions-worktree.set-1.json b/mobile/rpc-foundation/goldens/matrix-session.diff-review-actions-worktree.set-1.json index a6a79b93bd9..2c9c55021a0 100644 --- a/mobile/rpc-foundation/goldens/matrix-session.diff-review-actions-worktree.set-1.json +++ b/mobile/rpc-foundation/goldens/matrix-session.diff-review-actions-worktree.set-1.json @@ -56,40 +56,7 @@ "$rpc": "null" } }, - "1c6a13b5a4b5": { - "actionError": "", - "busyAction": { - "$rpc": "null" - }, - "screenState": { - "branchCompare": { - "$rpc": "null" - }, - "comments": [ - { - "body": "needs a test", - "createdAt": 0, - "filePath": "src/app.ts", - "id": "note-1", - "lineNumber": 4, - "side": "modified", - "worktreeId": "workspace-1" - } - ], - "kind": "ready", - "reviewState": { - "files": {}, - "version": 1 - }, - "status": { - "entries": [] - } - }, - "sendSheet": { - "$rpc": "null" - } - }, - "22df2e34a46c": { + "06005dec67d2": { "name": "worktree.set#1", "ordinal": 1, "args": [ @@ -111,6 +78,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": { @@ -152,17 +130,7 @@ } } }, - "32a7c0ae7918": { - "status": "rejected", - "startedAt": 0, - "settledAt": 0, - "error": { - "category": "Error", - "message": "outer refused", - "isRpcDeliveryUnknown": false - } - }, - "44fe127e6d75": { + "078c232ef420": { "name": "worktree.set#1", "ordinal": 1, "args": [ @@ -184,6 +152,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": { @@ -216,20 +195,17 @@ } ], "settlement": { - "status": "fulfilled", + "status": "rejected", "startedAt": 0, "settledAt": 0, - "value": { - "error": { - "code": "refused", - "message": "" - }, - "id": "frame-1", - "ok": false + "error": { + "category": "Error", + "message": "", + "isRpcDeliveryUnknown": true } } }, - "4701e4552237": { + "1a94a0989e43": { "name": "worktree.set#1", "ordinal": 1, "args": [ @@ -251,305 +227,7 @@ "worktreeId": "workspace-1" } ], - "mobileDiffReview": { - "completedAt": 1767225600000, - "files": { - "unstaged:src/app.ts": { - "filePath": "src/app.ts", - "key": "unstaged:src/app.ts", - "lastOpenedAt": { - "$rpc": "undefined" - }, - "lastSeenDiffIdentity": "identity-1", - "oldPath": { - "$rpc": "undefined" - }, - "reviewDiffIdentity": "identity-1", - "reviewedAt": 1767225600000, - "scope": "unstaged" - } - }, - "updatedAt": 1767225600000, - "version": 1 - }, - "worktree": "id:workspace-1" - } - }, - { - "name": "options", - "value": { - "$rpc": "absent" - } - } - ], - "settlement": { - "status": "fulfilled", - "startedAt": 0, - "settledAt": 0, - "value": { - "id": "frame-1", - "ok": true, - "result": { - "error": "refused" - } - } - } - }, - "63639602640e": { - "status": "rejected", - "startedAt": 0, - "settledAt": 0, - "error": { - "category": "Error", - "message": "Failed to save review state", - "isRpcDeliveryUnknown": false - } - }, - "73f7bfbf2ed4": { - "actionError": "Unknown method", - "busyAction": { - "$rpc": "null" - }, - "screenState": { - "branchCompare": { - "$rpc": "null" - }, - "comments": [ - { - "body": "needs a test", - "createdAt": 0, - "filePath": "src/app.ts", - "id": "note-1", - "lineNumber": 4, - "side": "modified", - "worktreeId": "workspace-1" - } - ], - "kind": "ready", - "reviewState": { - "files": {}, - "version": 1 - }, - "status": { - "entries": [] - } - }, - "sendSheet": { - "$rpc": "null" - } - }, - "86fb9ca891aa": { - "name": "worktree.set#1", - "ordinal": 1, - "args": [ - { - "name": "method", - "value": "worktree.set" - }, - { - "name": "params", - "value": { - "diffComments": [ - { - "body": "needs a test", - "createdAt": 0, - "filePath": "src/app.ts", - "id": "note-1", - "lineNumber": 4, - "side": "modified", - "worktreeId": "workspace-1" - } - ], - "mobileDiffReview": { - "completedAt": 1767225600000, - "files": { - "unstaged:src/app.ts": { - "filePath": "src/app.ts", - "key": "unstaged:src/app.ts", - "lastOpenedAt": { - "$rpc": "undefined" - }, - "lastSeenDiffIdentity": "identity-1", - "oldPath": { - "$rpc": "undefined" - }, - "reviewDiffIdentity": "identity-1", - "reviewedAt": 1767225600000, - "scope": "unstaged" - } - }, - "updatedAt": 1767225600000, - "version": 1 - }, - "worktree": "id:workspace-1" - } - }, - { - "name": "options", - "value": { - "$rpc": "absent" - } - } - ], - "settlement": { - "status": "fulfilled", - "startedAt": 0, - "settledAt": 0, - "value": { - "id": "frame-1", - "ok": true, - "result": { - "$rpc": "null" - } - } - } - }, - "9e0132f8d584": { - "actionError": "outer refused", - "busyAction": { - "$rpc": "null" - }, - "screenState": { - "branchCompare": { - "$rpc": "null" - }, - "comments": [ - { - "body": "needs a test", - "createdAt": 0, - "filePath": "src/app.ts", - "id": "note-1", - "lineNumber": 4, - "side": "modified", - "worktreeId": "workspace-1" - } - ], - "kind": "ready", - "reviewState": { - "files": {}, - "version": 1 - }, - "status": { - "entries": [] - } - }, - "sendSheet": { - "$rpc": "null" - } - }, - "a947768bc0ed": { - "status": "rejected", - "startedAt": 0, - "settledAt": 0, - "error": { - "category": "Error", - "message": "transport failure", - "isRpcDeliveryUnknown": true - } - }, - "b19741dc6694": { - "name": "worktree.set#1", - "ordinal": 1, - "args": [ - { - "name": "method", - "value": "worktree.set" - }, - { - "name": "params", - "value": { - "diffComments": [ - { - "body": "needs a test", - "createdAt": 0, - "filePath": "src/app.ts", - "id": "note-1", - "lineNumber": 4, - "side": "modified", - "worktreeId": "workspace-1" - } - ], - "mobileDiffReview": { - "completedAt": 1767225600000, - "files": { - "unstaged:src/app.ts": { - "filePath": "src/app.ts", - "key": "unstaged:src/app.ts", - "lastOpenedAt": { - "$rpc": "undefined" - }, - "lastSeenDiffIdentity": "identity-1", - "oldPath": { - "$rpc": "undefined" - }, - "reviewDiffIdentity": "identity-1", - "reviewedAt": 1767225600000, - "scope": "unstaged" - } - }, - "updatedAt": 1767225600000, - "version": 1 - }, - "worktree": "id:workspace-1" - } - }, - { - "name": "options", - "value": { - "$rpc": "absent" - } - } - ], - "settlement": { - "status": "fulfilled", - "startedAt": 0, - "settledAt": 0, - "value": { - "id": "frame-1", - "ok": true, - "result": { - "updated": true - } - } - } - }, - "b948e8307e81": { - "status": "rejected", - "startedAt": 0, - "settledAt": 0, - "error": { - "category": "Error", - "message": "Unknown method", - "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}}}" - }, - "c7584e82c72f": { - "status": "rejected", - "startedAt": 0, - "settledAt": 0, - "error": { - "category": "Error", - "message": "", - "isRpcDeliveryUnknown": true - } - }, - "d7908c1db91a": { - "name": "worktree.set#1", - "ordinal": 1, - "args": [ - { - "name": "method", - "value": "worktree.set" - }, - { - "name": "params", - "value": { - "diffComments": [ + "diffCommentsBase": [ { "body": "needs a test", "createdAt": 0, @@ -605,8 +283,8 @@ } } }, - "d98361dedc14": { - "actionError": "Failed to save review state", + "1c6a13b5a4b5": { + "actionError": "", "busyAction": { "$rpc": "null" }, @@ -638,7 +316,17 @@ "$rpc": "null" } }, - "e689b4c26c6e": { + "32a7c0ae7918": { + "status": "rejected", + "startedAt": 0, + "settledAt": 0, + "error": { + "category": "Error", + "message": "outer refused", + "isRpcDeliveryUnknown": false + } + }, + "387918d6d66f": { "name": "worktree.set#1", "ordinal": 1, "args": [ @@ -660,127 +348,7 @@ "worktreeId": "workspace-1" } ], - "mobileDiffReview": { - "completedAt": 1767225600000, - "files": { - "unstaged:src/app.ts": { - "filePath": "src/app.ts", - "key": "unstaged:src/app.ts", - "lastOpenedAt": { - "$rpc": "undefined" - }, - "lastSeenDiffIdentity": "identity-1", - "oldPath": { - "$rpc": "undefined" - }, - "reviewDiffIdentity": "identity-1", - "reviewedAt": 1767225600000, - "scope": "unstaged" - } - }, - "updatedAt": 1767225600000, - "version": 1 - }, - "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 - } - } - }, - "e779df686927": { - "name": "worktree.set#1", - "ordinal": 1, - "args": [ - { - "name": "method", - "value": "worktree.set" - }, - { - "name": "params", - "value": { - "diffComments": [ - { - "body": "needs a test", - "createdAt": 0, - "filePath": "src/app.ts", - "id": "note-1", - "lineNumber": 4, - "side": "modified", - "worktreeId": "workspace-1" - } - ], - "mobileDiffReview": { - "completedAt": 1767225600000, - "files": { - "unstaged:src/app.ts": { - "filePath": "src/app.ts", - "key": "unstaged:src/app.ts", - "lastOpenedAt": { - "$rpc": "undefined" - }, - "lastSeenDiffIdentity": "identity-1", - "oldPath": { - "$rpc": "undefined" - }, - "reviewDiffIdentity": "identity-1", - "reviewedAt": 1767225600000, - "scope": "unstaged" - } - }, - "updatedAt": 1767225600000, - "version": 1 - }, - "worktree": "id:workspace-1" - } - }, - { - "name": "options", - "value": { - "$rpc": "absent" - } - } - ], - "settlement": { - "status": "rejected", - "startedAt": 0, - "settledAt": 0, - "error": { - "category": "Error", - "message": "transport failure", - "isRpcDeliveryUnknown": true - } - } - }, - "e7feb1eb0a9a": { - "name": "worktree.set#1", - "ordinal": 1, - "args": [ - { - "name": "method", - "value": "worktree.set" - }, - { - "name": "params", - "value": { - "diffComments": [ + "diffCommentsBase": [ { "body": "needs a test", "createdAt": 0, @@ -838,15 +406,7 @@ } } }, - "eb79a9b3682a": { - "status": "fulfilled", - "startedAt": 0, - "settledAt": 0, - "value": { - "$rpc": "undefined" - } - }, - "f19c54834931": { + "45f8c93797f0": { "name": "worktree.set#1", "ordinal": 1, "args": [ @@ -868,6 +428,143 @@ "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": { + "unstaged:src/app.ts": { + "filePath": "src/app.ts", + "key": "unstaged:src/app.ts", + "lastOpenedAt": { + "$rpc": "undefined" + }, + "lastSeenDiffIdentity": "identity-1", + "oldPath": { + "$rpc": "undefined" + }, + "reviewDiffIdentity": "identity-1", + "reviewedAt": 1767225600000, + "scope": "unstaged" + } + }, + "updatedAt": 1767225600000, + "version": 1 + }, + "worktree": "id:workspace-1" + } + }, + { + "name": "options", + "value": { + "$rpc": "absent" + } + } + ], + "settlement": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "error": { + "code": "refused", + "message": "" + }, + "id": "frame-1", + "ok": false + } + } + }, + "63639602640e": { + "status": "rejected", + "startedAt": 0, + "settledAt": 0, + "error": { + "category": "Error", + "message": "Failed to save review state", + "isRpcDeliveryUnknown": false + } + }, + "73f7bfbf2ed4": { + "actionError": "Unknown method", + "busyAction": { + "$rpc": "null" + }, + "screenState": { + "branchCompare": { + "$rpc": "null" + }, + "comments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "worktreeId": "workspace-1" + } + ], + "kind": "ready", + "reviewState": { + "files": {}, + "version": 1 + }, + "status": { + "entries": [] + } + }, + "sendSheet": { + "$rpc": "null" + } + }, + "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}}}" + }, + "9763741966f9": { + "name": "worktree.set#1", + "ordinal": 1, + "args": [ + { + "name": "method", + "value": "worktree.set" + }, + { + "name": "params", + "value": { + "diffComments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "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": { @@ -913,7 +610,50 @@ } } }, - "f2507f6faff8": { + "9e0132f8d584": { + "actionError": "outer refused", + "busyAction": { + "$rpc": "null" + }, + "screenState": { + "branchCompare": { + "$rpc": "null" + }, + "comments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "worktreeId": "workspace-1" + } + ], + "kind": "ready", + "reviewState": { + "files": {}, + "version": 1 + }, + "status": { + "entries": [] + } + }, + "sendSheet": { + "$rpc": "null" + } + }, + "a947768bc0ed": { + "status": "rejected", + "startedAt": 0, + "settledAt": 0, + "error": { + "category": "Error", + "message": "transport failure", + "isRpcDeliveryUnknown": true + } + }, + "aa4104b74da8": { "name": "worktree.set#1", "ordinal": 1, "args": [ @@ -935,6 +675,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": { @@ -972,11 +723,381 @@ "settledAt": 0, "error": { "category": "Error", - "message": "", + "message": "transport failure", "isRpcDeliveryUnknown": true } } }, + "b948e8307e81": { + "status": "rejected", + "startedAt": 0, + "settledAt": 0, + "error": { + "category": "Error", + "message": "Unknown method", + "isRpcDeliveryUnknown": false + } + }, + "c7584e82c72f": { + "status": "rejected", + "startedAt": 0, + "settledAt": 0, + "error": { + "category": "Error", + "message": "", + "isRpcDeliveryUnknown": true + } + }, + "d98361dedc14": { + "actionError": "Failed to save review state", + "busyAction": { + "$rpc": "null" + }, + "screenState": { + "branchCompare": { + "$rpc": "null" + }, + "comments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "worktreeId": "workspace-1" + } + ], + "kind": "ready", + "reviewState": { + "files": {}, + "version": 1 + }, + "status": { + "entries": [] + } + }, + "sendSheet": { + "$rpc": "null" + } + }, + "df1198f920a7": { + "name": "worktree.set#1", + "ordinal": 1, + "args": [ + { + "name": "method", + "value": "worktree.set" + }, + { + "name": "params", + "value": { + "diffComments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "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": { + "unstaged:src/app.ts": { + "filePath": "src/app.ts", + "key": "unstaged:src/app.ts", + "lastOpenedAt": { + "$rpc": "undefined" + }, + "lastSeenDiffIdentity": "identity-1", + "oldPath": { + "$rpc": "undefined" + }, + "reviewDiffIdentity": "identity-1", + "reviewedAt": 1767225600000, + "scope": "unstaged" + } + }, + "updatedAt": 1767225600000, + "version": 1 + }, + "worktree": "id:workspace-1" + } + }, + { + "name": "options", + "value": { + "$rpc": "absent" + } + } + ], + "settlement": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "id": "frame-1", + "ok": true, + "result": { + "error": "refused" + } + } + } + }, + "df876725e770": { + "name": "worktree.set#1", + "ordinal": 1, + "args": [ + { + "name": "method", + "value": "worktree.set" + }, + { + "name": "params", + "value": { + "diffComments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "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": { + "unstaged:src/app.ts": { + "filePath": "src/app.ts", + "key": "unstaged:src/app.ts", + "lastOpenedAt": { + "$rpc": "undefined" + }, + "lastSeenDiffIdentity": "identity-1", + "oldPath": { + "$rpc": "undefined" + }, + "reviewDiffIdentity": "identity-1", + "reviewedAt": 1767225600000, + "scope": "unstaged" + } + }, + "updatedAt": 1767225600000, + "version": 1 + }, + "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 + } + } + }, + "e0def05e4021": { + "name": "worktree.set#1", + "ordinal": 1, + "args": [ + { + "name": "method", + "value": "worktree.set" + }, + { + "name": "params", + "value": { + "diffComments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "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": { + "unstaged:src/app.ts": { + "filePath": "src/app.ts", + "key": "unstaged:src/app.ts", + "lastOpenedAt": { + "$rpc": "undefined" + }, + "lastSeenDiffIdentity": "identity-1", + "oldPath": { + "$rpc": "undefined" + }, + "reviewDiffIdentity": "identity-1", + "reviewedAt": 1767225600000, + "scope": "unstaged" + } + }, + "updatedAt": 1767225600000, + "version": 1 + }, + "worktree": "id:workspace-1" + } + }, + { + "name": "options", + "value": { + "$rpc": "absent" + } + } + ], + "settlement": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "id": "frame-1", + "ok": true, + "result": { + "$rpc": "null" + } + } + } + }, + "eb79a9b3682a": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "$rpc": "undefined" + } + }, + "ebfe5f82347b": { + "name": "worktree.set#1", + "ordinal": 1, + "args": [ + { + "name": "method", + "value": "worktree.set" + }, + { + "name": "params", + "value": { + "diffComments": [ + { + "body": "needs a test", + "createdAt": 0, + "filePath": "src/app.ts", + "id": "note-1", + "lineNumber": 4, + "side": "modified", + "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": { + "unstaged:src/app.ts": { + "filePath": "src/app.ts", + "key": "unstaged:src/app.ts", + "lastOpenedAt": { + "$rpc": "undefined" + }, + "lastSeenDiffIdentity": "identity-1", + "oldPath": { + "$rpc": "undefined" + }, + "reviewDiffIdentity": "identity-1", + "reviewedAt": 1767225600000, + "scope": "unstaged" + } + }, + "updatedAt": 1767225600000, + "version": 1 + }, + "worktree": "id:workspace-1" + } + }, + { + "name": "options", + "value": { + "$rpc": "absent" + } + } + ], + "settlement": { + "status": "fulfilled", + "startedAt": 0, + "settledAt": 0, + "value": { + "id": "frame-1", + "ok": true, + "result": { + "updated": true + } + } + } + }, "fa4af2435d85": { "actionError": "transport failure", "busyAction": { @@ -1017,8 +1138,8 @@ { "id": "review-mark-reviewed-persists.normal:persisted", "observation": { - "sender": ["b19741dc6694"], - "payloads": ["bc47263d2197"], + "sender": ["ebfe5f82347b"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "eb79a9b3682a" }, @@ -1029,8 +1150,8 @@ { "id": "review-mark-reviewed-persists.result-absent:persisted", "observation": { - "sender": ["22df2e34a46c"], - "payloads": ["bc47263d2197"], + "sender": ["06005dec67d2"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "eb79a9b3682a" }, @@ -1041,8 +1162,8 @@ { "id": "review-mark-reviewed-persists.result-null:persisted", "observation": { - "sender": ["86fb9ca891aa"], - "payloads": ["bc47263d2197"], + "sender": ["e0def05e4021"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "eb79a9b3682a" }, @@ -1053,8 +1174,8 @@ { "id": "review-mark-reviewed-persists.inner-ok-missing:persisted", "observation": { - "sender": ["4701e4552237"], - "payloads": ["bc47263d2197"], + "sender": ["df1198f920a7"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "eb79a9b3682a" }, @@ -1065,8 +1186,8 @@ { "id": "review-mark-reviewed-persists.inner-false-string-error:persisted", "observation": { - "sender": ["d7908c1db91a"], - "payloads": ["bc47263d2197"], + "sender": ["1a94a0989e43"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "eb79a9b3682a" }, @@ -1077,8 +1198,8 @@ { "id": "review-mark-reviewed-persists.inner-false-object-error:persisted", "observation": { - "sender": ["e7feb1eb0a9a"], - "payloads": ["bc47263d2197"], + "sender": ["387918d6d66f"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "eb79a9b3682a" }, @@ -1089,8 +1210,8 @@ { "id": "review-mark-reviewed-persists.outer-refused:persisted", "observation": { - "sender": ["f19c54834931"], - "payloads": ["bc47263d2197"], + "sender": ["9763741966f9"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "32a7c0ae7918" }, @@ -1101,8 +1222,8 @@ { "id": "review-mark-reviewed-persists.outer-refused-no-message:persisted", "observation": { - "sender": ["44fe127e6d75"], - "payloads": ["bc47263d2197"], + "sender": ["45f8c93797f0"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "63639602640e" }, @@ -1113,8 +1234,8 @@ { "id": "review-mark-reviewed-persists.method-not-found:persisted", "observation": { - "sender": ["e689b4c26c6e"], - "payloads": ["bc47263d2197"], + "sender": ["df876725e770"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "b948e8307e81" }, @@ -1125,8 +1246,8 @@ { "id": "review-mark-reviewed-persists.transport-rejection:persisted", "observation": { - "sender": ["e779df686927"], - "payloads": ["bc47263d2197"], + "sender": ["aa4104b74da8"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "a947768bc0ed" }, @@ -1137,8 +1258,8 @@ { "id": "review-mark-reviewed-persists.transport-rejection-no-message:persisted", "observation": { - "sender": ["f2507f6faff8"], - "payloads": ["bc47263d2197"], + "sender": ["078c232ef420"], + "payloads": ["81c9edeaad86"], "settlements": { "mark-reviewed": "c7584e82c72f" }, diff --git a/mobile/rpc-foundation/goldens/review-mark-reviewed-persists.json b/mobile/rpc-foundation/goldens/review-mark-reviewed-persists.json index 755ca882f77..bee98a71364 100644 --- a/mobile/rpc-foundation/goldens/review-mark-reviewed-persists.json +++ b/mobile/rpc-foundation/goldens/review-mark-reviewed-persists.json @@ -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" }, diff --git a/mobile/rpc-foundation/goldens/review-mark-reviewed-rolls-back.json b/mobile/rpc-foundation/goldens/review-mark-reviewed-rolls-back.json index a7bdebb2830..5e4c790f2e9 100644 --- a/mobile/rpc-foundation/goldens/review-mark-reviewed-rolls-back.json +++ b/mobile/rpc-foundation/goldens/review-mark-reviewed-rolls-back.json @@ -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" }, diff --git a/mobile/rpc-foundation/goldens/session-diff-notes-load-refused.json b/mobile/rpc-foundation/goldens/session-diff-notes-load-refused.json index 586afc30c3a..a77512d0b4b 100644 --- a/mobile/rpc-foundation/goldens/session-diff-notes-load-refused.json +++ b/mobile/rpc-foundation/goldens/session-diff-notes-load-refused.json @@ -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" diff --git a/mobile/rpc-foundation/goldens/session-diff-notes-loaded.json b/mobile/rpc-foundation/goldens/session-diff-notes-loaded.json index c83017fae64..6ef5fae55a0 100644 --- a/mobile/rpc-foundation/goldens/session-diff-notes-loaded.json +++ b/mobile/rpc-foundation/goldens/session-diff-notes-loaded.json @@ -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" diff --git a/mobile/rpc-foundation/pilot-scenarios.json b/mobile/rpc-foundation/pilot-scenarios.json index ede0fa55e2a..2fff0ffed11 100644 --- a/mobile/rpc-foundation/pilot-scenarios.json +++ b/mobile/rpc-foundation/pilot-scenarios.json @@ -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": { diff --git a/mobile/src/session/mobile-review-notes-host-merge.test.tsx b/mobile/src/session/mobile-review-notes-host-merge.test.tsx new file mode 100644 index 00000000000..f5df89d72d4 --- /dev/null +++ b/mobile/src/session/mobile-review-notes-host-merge.test.tsx @@ -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 => { + 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(() => {}) + }) + const client: RpcClient = { + sendRequest, + subscribe: () => () => {}, + updateTerminalSubscriptionViewport: () => {}, + getState: () => 'connected', + getReconnectAttempt: () => 0, + getLastConnectedAt: () => null, + onStateChange: () => () => {}, + notifyForeground: () => {}, + close: () => {} + } + return { host, client, heldShows, heldSets, sendRequest } +} + +type Notes = ReturnType +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 { + function Harness() { + const [diffComments, setDiffComments] = useState([]) + 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 = Promise.resolve(false) + let clearing: Promise = 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 | 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']) + }) +}) diff --git a/mobile/src/session/mobile-review-notes-sync.ts b/mobile/src/session/mobile-review-notes-sync.ts new file mode 100644 index 00000000000..0db42a73d13 --- /dev/null +++ b/mobile/src/session/mobile-review-notes-sync.ts @@ -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 +} + +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( + sync: ReviewNotesSync, + run: () => Promise +): Promise { + const result = sync.queue.then(run) + sync.queue = result.catch(() => undefined) + return result +} diff --git a/mobile/src/session/mobile-session-diff-notes-new-agent-send.test.ts b/mobile/src/session/mobile-session-diff-notes-new-agent-send.test.ts index f199e1e68e6..b9094fe25fa 100644 --- a/mobile/src/session/mobile-session-diff-notes-new-agent-send.test.ts +++ b/mobile/src/session/mobile-session-diff-notes-new-agent-send.test.ts @@ -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((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() diff --git a/mobile/src/session/mobile-session-write-operations.ts b/mobile/src/session/mobile-session-write-operations.ts index d1aa47714f5..1e247b0d168 100644 --- a/mobile/src/session/mobile-session-write-operations.ts +++ b/mobile/src/session/mobile-session-write-operations.ts @@ -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)) }) ) diff --git a/mobile/src/session/use-mobile-diff-review-comment-actions.ts b/mobile/src/session/use-mobile-diff-review-comment-actions.ts index b5e2df91e16..eeeaa58fee9 100644 --- a/mobile/src/session/use-mobile-diff-review-comment-actions.ts +++ b/mobile/src/session/use-mobile-diff-review-comment-actions.ts @@ -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( diff --git a/mobile/src/session/use-mobile-session-diff-comments.ts b/mobile/src/session/use-mobile-session-diff-comments.ts index b502959c8ec..758c93723ad 100644 --- a/mobile/src/session/use-mobile-session-diff-comments.ts +++ b/mobile/src/session/use-mobile-session-diff-comments.ts @@ -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>( sendingDiffCommentIdsRef.current ) - const loadDiffComments = useCallback(async (): Promise => { - 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 => { - 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 => { + 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 => { @@ -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, diff --git a/src/main/persistence/loading-store/metadata-lineage-operations.ts b/src/main/persistence/loading-store/metadata-lineage-operations.ts index 83c689e1a9e..8a8d063c433 100644 --- a/src/main/persistence/loading-store/metadata-lineage-operations.ts +++ b/src/main/persistence/loading-store/metadata-lineage-operations.ts @@ -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 & WorkspaceAttachmentMutation + meta: Partial & WorkspaceAttachmentMutation & DiffCommentBaseMutation ): WorktreeMeta { return setWorktreeMetaForHostOperation( this[metadataLineageOperationsContext].runtime, diff --git a/src/main/persistence/loading-store/worktree-identity-metadata.ts b/src/main/persistence/loading-store/worktree-identity-metadata.ts index b0a0c44777e..8bdc665ad09 100644 --- a/src/main/persistence/loading-store/worktree-identity-metadata.ts +++ b/src/main/persistence/loading-store/worktree-identity-metadata.ts @@ -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 & WorkspaceAttachmentMutation + meta: Partial & WorkspaceAttachmentMutation & DiffCommentBaseMutation ): WorktreeMeta { const state = runtime.state const executionHostId = worktreeMetaHostFor(worktreeId, requestedHostId) diff --git a/src/main/persistence/loading-store/worktree-meta-write-normalization.test.ts b/src/main/persistence/loading-store/worktree-meta-write-normalization.test.ts index 206ce390822..18cc224497e 100644 --- a/src/main/persistence/loading-store/worktree-meta-write-normalization.test.ts +++ b/src/main/persistence/loading-store/worktree-meta-write-normalization.test.ts @@ -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] diff --git a/src/main/persistence/loading-store/worktree-meta-write-normalization.ts b/src/main/persistence/loading-store/worktree-meta-write-normalization.ts index 15e4e53a60e..08add1011d9 100644 --- a/src/main/persistence/loading-store/worktree-meta-write-normalization.ts +++ b/src/main/persistence/loading-store/worktree-meta-write-normalization.ts @@ -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 & WorkspaceAttachmentMutation, + updates: Partial & 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), diff --git a/src/main/runtime/rpc/methods/worktree-set-method.test.ts b/src/main/runtime/rpc/methods/worktree-set-method.test.ts new file mode 100644 index 00000000000..9db338af1f9 --- /dev/null +++ b/src/main/runtime/rpc/methods/worktree-set-method.test.ts @@ -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: [] }) + ) + }) +}) diff --git a/src/main/runtime/rpc/methods/worktree-set-method.ts b/src/main/runtime/rpc/methods/worktree-set-method.ts new file mode 100644 index 00000000000..e1d0c10497b --- /dev/null +++ b/src/main/runtime/rpc/methods/worktree-set-method.ts @@ -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] } : {}) } + } +}) diff --git a/src/main/runtime/rpc/methods/worktree.ts b/src/main/runtime/rpc/methods/worktree.ts index 04cffcd3ef6..696b784d1b2 100644 --- a/src/main/runtime/rpc/methods/worktree.ts +++ b/src/main/runtime/rpc/methods/worktree.ts @@ -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', diff --git a/src/shared/diff-comment-mutation.ts b/src/shared/diff-comment-mutation.ts new file mode 100644 index 00000000000..599e7f4e592 --- /dev/null +++ b/src/shared/diff-comment-mutation.ts @@ -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] +} diff --git a/src/shared/rpc-contract/worktree-params.ts b/src/shared/rpc-contract/worktree-params.ts index 3bd02841683..49f1eeb39d5 100644 --- a/src/shared/rpc-contract/worktree-params.ts +++ b/src/shared/rpc-contract/worktree-params.ts @@ -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