You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@maka.apache.org by "liuxiaocs7 (via GitHub)" <gi...@apache.org> on 2026/08/24 03:36:40 UTC

[PR] fix(desktop): snapshot linked children for side conversations [maka]

liuxiaocs7 opened a new pull request, #3669:
URL: https://github.com/apache/maka/pull/3669

   ## Summary
   
   - carry an explicit Side Conversation intent from the renderer through Desktop IPC and the Runtime Host protocol
   - snapshot terminal linked-child results without retaining child-session ownership or control identifiers
   - copy and rewrite referenced artifacts, drop child task-ledger ownership, and preserve ordinary branch/revision safety rules
   - surface structured `session_busy` and `operation_unavailable` outcomes, retrying busy sources from the latest settled turn
   
   Closes #3654
   
   ## Verification
   
   - built `@maka/core`, `storage`, `runtime`, and `runtime-host`
   - typechecked core, storage, runtime, runtime-host, and Desktop targets
   - ran Biome checks across all changed files
   - passed targeted conversation-copy, graph-reference, protocol, artifact, task-ledger, renderer retry/cleanup/disposal, Desktop IPC, and two-client UDS integration tests
   - rebased onto current `origin/main` and ran `git diff --check origin/main...HEAD`
   
   ## AI use
   
   Select exactly one:
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: Codex contributed implementation, tests, verification, and review; Claude performed an additional review of the completed change.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — described under Summary above
   - [ ] No
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): snapshot linked children for side conversations [maka]

Posted by "ARE404 (via GitHub)" <gi...@apache.org>.

ARE404 commented on code in PR #3669:
URL: https://github.com/apache/maka/pull/3669#discussion_r3840960227


##########
apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts:
##########
@@ -298,9 +299,19 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan
       })
         .then((result) => {
           if (result.status === 'ready' && mountedRef.current) {
+            setForkRetryPending(false);
+            setError(null);
             commitFork(result.session);
           } else if (result.status === 'error' && mountedRef.current) {
-            setError(copyRef.current.errors.forkSetupFailed);
+            setForkRetryPending(result.code === 'fork_source_busy');

Review Comment:
   **[P3] Add the CONTRIBUTING-required before/after UI evidence.** This branch introduces user-visible busy/unsupported banners, automatic retry state, and clearing of a stale banner on success (`:300-362`), while the PR body contains no screenshot or recording. The repository standard says UI changes must include before/after screenshots or a recording; the story/test evidence proves behavior but does not show reviewers the rendered states.



##########
packages/runtime-host/src/protocol/session-revision.ts:
##########
@@ -40,6 +46,7 @@ export interface SessionConversationCopyInput {
   readonly targetSessionId: string;
   readonly sourceTurnId: string;
   readonly expectedSourceRevision: number;
+  readonly intent?: 'side_conversation';

Review Comment:
   **[P3] Give copy purpose one discriminated domain representation.** The same concept is currently `sideConversation: true` in Renderer ports, optional `intent` here and in durable/cleanup records, `ConversationCopySemanticKind` in Host, and `mode:side_conversation` as a label. Host then reconstructs a third kind and repeats `kind === 'side_conversation'` through validation, artifact policy, ledger policy, labels, and fingerprinting. A shared `branch | revision | side_conversation` copy mode (with a legacy decode adapter if stored compatibility requires it) would make the interface truthful and centralize the derived label/policy; roughly 20 conversion/comparison branches across six production files become deletable. This is design debt, not a correctness blocker.



##########
packages/runtime/src/conversation-copy.ts:
##########
@@ -87,6 +87,11 @@ export type ConversationCopyArtifactReferenceMap =
       readonly relativePaths: ReadonlyMap<string, string>;
       readonly linkedChildren:
         | { readonly mode: 'reject' }
+        | {
+            readonly mode: 'snapshot';
+            readonly archivedResults: ReadonlyMap<string, string>;
+            readonly artifactIds: ReadonlyMap<string, string>;

Review Comment:
   **[P3] Delete the nested `artifactIds` map.** In exact mode the outer object already carries the source→target artifact map at `:86`; Host passes that exact same `artifactCopy.artifactIds` value both there and here. `rewriteSnapshotArtifactIds` can read `references.artifactIds` directly once it has established `linkedChildren.mode === 'snapshot'`. Removing this field eliminates a duplicate representation and prevents the two maps from ever drifting.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): snapshot linked children for side conversations [maka]

Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.

liuxiaocs7 commented on PR #3669:
URL: https://github.com/apache/maka/pull/3669#issuecomment-5392664711

   Fixed the retry flicker in `af3b5bc92`. The 2-second busy retry loop no longer re-enters the foreground `preparing` state, so the panel keeps the actionable busy banner and composer stable while retrying in the background. The initial creation still shows the preparing state, and a successful retry clears the banner. Added a regression assertion that holds a retry in flight and verifies `preparing=false`; focused Desktop tests pass.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): snapshot linked children for side conversations [maka]

Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.

liuxiaocs7 commented on code in PR #3669:
URL: https://github.com/apache/maka/pull/3669#discussion_r3841745544


##########
apps/desktop/src/renderer/features/workbar/tools/side-chat/use-quote-companion.ts:
##########
@@ -298,9 +299,19 @@ export function useQuoteCompanion(input: UseQuoteCompanionInput): UseQuoteCompan
       })
         .then((result) => {
           if (result.status === 'ready' && mountedRef.current) {
+            setForkRetryPending(false);
+            setError(null);
             commitFork(result.session);
           } else if (result.status === 'error' && mountedRef.current) {
-            setError(copyRef.current.errors.forkSetupFailed);
+            setForkRetryPending(result.code === 'fork_source_busy');

Review Comment:
   The existing UI failure screenshot is already attached to the linked issue #3654: https://github.com/apache/maka/issues/3654. Per the author direction, we are keeping that evidence there rather than duplicating it in this PR. No additional code change is needed for this thread.



##########
packages/runtime-host/src/protocol/session-revision.ts:
##########
@@ -40,6 +46,7 @@ export interface SessionConversationCopyInput {
   readonly targetSessionId: string;
   readonly sourceTurnId: string;
   readonly expectedSourceRevision: number;
+  readonly intent?: 'side_conversation';

Review Comment:
   Agreed that a single discriminated copy-purpose representation would be a useful cleanup. It spans Renderer, Desktop IPC, Host protocol, durable cleanup records, and compatibility decoding, so I am leaving this non-blocking P3 out of the focused bug-fix PR rather than expanding its migration surface. The current representation remains strictly decoded and included in the idempotency fingerprint.



##########
packages/runtime/src/conversation-copy.ts:
##########
@@ -87,6 +87,11 @@ export type ConversationCopyArtifactReferenceMap =
       readonly relativePaths: ReadonlyMap<string, string>;
       readonly linkedChildren:
         | { readonly mode: 'reject' }
+        | {
+            readonly mode: 'snapshot';
+            readonly archivedResults: ReadonlyMap<string, string>;
+            readonly artifactIds: ReadonlyMap<string, string>;

Review Comment:
   Addressed in `a21b59922`. Snapshot mode no longer carries a nested `artifactIds` alias; `rewriteSnapshotArtifactIds` now reads the outer exact `references.artifactIds` map after checking the snapshot discriminant, and the focused tests were updated.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org

Re: [PR] fix(desktop): snapshot linked children for side conversations [maka]

Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.

liuxiaocs7 commented on code in PR #3669:
URL: https://github.com/apache/maka/pull/3669#discussion_r3842204472


##########
packages/runtime/src/conversation-copy.ts:
##########
@@ -1124,12 +1146,21 @@ function rewriteToolResultContent(
     };
   }
   if (content.kind === 'json' && isArchivedToolResultPlaceholder(content.value)) {
+    const snapshot = rewriteArchivedSnapshot(content.value, references);
+    if (snapshot) return snapshot;
     return {
       ...content,
       value: rewriteArchivedToolResult(content.value, references),
     };
   }
   if (content.kind === 'subagent') {
+    if (linkedChildrenAreSnapshots(references)) {
+      const { childSessionId: _childSessionId, runId: _runId, ...snapshot } = content;

Review Comment:
   已在 `6551f9b8f` 修复。snapshot 重写现在按 `childSessionId` 区分：真正的 linked child 才会剥离 child/run/resume identity；没有 `childSessionId` 的 source-owned `subagent` / `agent_swarm` 继续通过现有 run/artifact 映射重写。新增了两条 side-mode 回归测试，覆盖 `runId`、`resumedFromRunId` 和 artifact 映射；`conversation-copy` 17/17、Runtime typecheck、Biome 与 `git diff --check` 均通过。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org