feat: adopt and fork native harness sessions by qiankunli · Pull Request #231 · compforge/baton

qiankunli · 2026-07-30T12:26:43Z

Allow resume and fork to accept Codex and Claude Code native session IDs,
with read-only auto-detection and explicit cx:/cc: disambiguation.

Reconstruct native user/assistant history as ordinary Baton turns, bind the
native session under a single Baton owner, and preserve provenance for later
cross-Harness handoff and Baton-level forks.

Document the ownership model and guard against duplicate native bindings.

Treat Codex and Claude Code session IDs only as import references.
Resolve them read-only, materialize or reuse the source BatonSession,
then run the ordinary Baton resume or fork path.

Remove native fork operations from providers, keep fork children on
fresh HarnessSessions, and align tests and documentation with the
single lifecycle.

Serialize native-session ownership checks and creation across processes,
and persist provenance with the Harness binding in one atomic metadata
replacement.

Fail closed when bare-ID provider discovery is incomplete, distinguish
lookup failures from genuine absence, retain Codex stderr diagnostics,
and align fork errors with the shared CLI failure path.

Allow resume and fork to accept Codex and Claude Code native session IDs, with read-only auto-detection and explicit cx:/cc: disambiguation. Reconstruct native user/assistant history as ordinary Baton turns, bind the native session under a single Baton owner, and preserve provenance for later cross-Harness handoff and Baton-level forks. Document the ownership model and guard against duplicate native bindings.

qiankunli · 2026-07-30T12:35:25Z

+    seenCursors.add(nextCursor);
+    cursor = nextCursor;
+  }
+  return descending.toReversed();


🤖 devloop code-review · deepseek-v4-pro

Array.prototype.toReversed() requires Node.js 20+, but package.json specifies "node": ">=18". This will throw a runtime error when processing native session history on Node 18 or 19. Consider using .reverse() on a copy or [...descending].reverse() instead.

_{ccr:fp=18089dbd818b}

ccr:label=wrong — Baton 的 Node 入口只负责启动包内 Bun，native-session.ts 始终由 Bun 执行；Node >=18 不是该 TS 代码的运行时约束 #textbook

qiankunli · 2026-07-30T12:35:26Z

+  operation: (peer: CodexNativePeer) => Promise<T>,
+): Promise<T> {
+  const [command, ...args] = codexLaunchCommand(options.command);
+  const child = spawn(command as string, args, {


🤖 devloop code-review · deepseek-v4-pro

The as string type assertion on command is unnecessary since codexLaunchCommand returns string[], making command already typed as string. This assertion masks potential type issues if the return type changes.

_{ccr:fp=c51eb7b20f4a}

ccr:label=wrong — tsconfig 开启 noUncheckedIndexedAccess，string[] 解构出的 command 是 string | undefined；该断言用于表达 codexLaunchCommand 的非空运行时不变量 #textbook

qiankunli · 2026-07-30T12:35:27Z

+    });
+    return result.sessionId;
+  },
+};


🤖 devloop code-review · deepseek-v4-pro

The inspect and fork methods don't match the NativeSessionProvider interface contract. Both methods are missing the required options: NativeSessionProviderOptions parameter. The interface (native-session.ts:30-37) defines:

inspect(sessionId: string, options: NativeSessionProviderOptions): Promise<NativeSessionInfo | null>

fork(source: NativeSessionInfo, options: NativeSessionProviderOptions): Promise<string>

Callers in native-session.ts pass options at line 90 (inspectSource) and line 199 (forkNativeSession). Without this parameter, the Claude provider will fail TypeScript compilation and cause runtime argument mismatches. Compare with the Codex provider (src/harness/codex/native-session.ts) which correctly accepts both parameters.

_{ccr:fp=5dde89429559}

ccr:label=wrong — 当前 NativeSessionProvider 已无 fork 契约；inspect 实现少声明一个未使用参数在 TypeScript 中合法且已通过严格 typecheck #stale

qiankunli · 2026-07-30T12:35:28Z

+   */
+  createFromNativeSession(adoption: NativeSessionAdoption): SessionHandle {
+    const session = this.createSession({ cwd: adoption.cwd, title: adoption.title });
+    const label = adoption.title?.trim() || adoption.sourceSessionId;


🤖 devloop code-review · seed-2.1-pro

createFromNativeSession is a public method that binds a native session to a new BatonSession, but it does not perform its own duplicate-binding check. The guard exists only in the caller adoptNativeSession (src/harness/native-session.ts:177), creating a TOCTOU window between findByNativeSession and createFromNativeSession. While the practical risk is low in this single-process CLI, any future code path that calls createFromNativeSession directly (or two concurrent baton resume processes) would bypass the ownership guard and create duplicate BatonSessions for the same native session.

Consider either:

Moving the findByNativeSession check into createFromNativeSession itself so the method is self-protecting, or

Documenting on the method that callers must call findByNativeSession first.

_{ccr:fp=b0166a4a9c21}

ccr:label=important — 已将 duplicate check 和创建收进 SessionStore.materializeNativeSession，并以跨进程 O_EXCL 锁保护，移除 caller 侧 TOCTOU

qiankunli · 2026-07-30T12:35:31Z

🤖 devloop code-review · deepseek-v4-pro

Error handling inconsistency: the resume native path uses fail() while the fork native path uses console.error(...) + process.exit(1). Both achieve the same result (exit code 1), but the resume path's fail() is the established pattern used throughout this file. For consistency and maintainability, the fork catch block should also use fail().

_{ccr:fp=86230ef8f76e}

ccr:label=minor — 属于真实但不影响行为的一致性问题；fork 的两个错误出口均已改用既有 fail()

qiankunli · 2026-07-30T12:35:32Z

🤖 devloop code-review · origin/main..HEAD · cc1d8394d · models: deepseek-v4-pro×35, seed-2.1-pro×36, seed-2.1-turbo×34 · cost: 510s · ccr v1.9.0

5 finding(s)（5 条已作为独立 review thread 发布）

Treat Codex and Claude Code session IDs only as import references. Resolve them read-only, materialize or reuse the source BatonSession, then run the ordinary Baton resume or fork path. Remove native fork operations from providers, keep fork children on fresh HarnessSessions, and align tests and documentation with the single lifecycle.

qiankunli · 2026-07-30T12:39:50Z

+  NativeTranscriptEntry,
+} from "../native-session.ts";
+
+function textContent(value: unknown): string {


🤖 devloop code-review · deepseek-v4-pro

The textContent function uses unknown as the parameter type, discarding the type information from SessionMessage.message. The Claude Agent SDK's SessionMessage type likely has a structured message property with known content block shapes. Using unknown requires manual type guards throughout the function, undermining TypeScript's type safety. Consider using the SDK's actual content block type for the parameter.

_{ccr:fp=c9cc7427bc27}

ccr:label=wrong — Claude Agent SDK 的 SessionMessage.message 在已安装 sdk.d.ts 中明确声明为 unknown，当前边界守卫没有丢失可用类型信息 #cross-file

qiankunli · 2026-07-30T12:39:52Z

+  NativeSessionInfo,
+  NativeSessionProvider,
+  NativeTranscriptEntry,
+} from "../native-session.ts";


🤖 devloop code-review · deepseek-v4-pro

The import is missing NativeSessionProviderOptions, which is needed for the inspect method's options parameter to match the NativeSessionProvider interface contract.

_{ccr:fp=d077a43adcea}

ccr:label=wrong — inspect 不使用 options，TypeScript 允许实现省略尾部参数；严格 typecheck 已证明契约成立，额外导入会成为未使用噪音 #textbook

qiankunli · 2026-07-30T12:39:54Z

🤖 devloop code-review · seed-2.1-pro

Prior review flag (cc1d839) still present: the fork catch block uses console.error(...) + process.exit(1) while the resume catch block (line 202-203) uses fail(). Both achieve exit code 1, but fail() is the established pattern throughout this file (used in runLogs, runPlugins, and the resume path). For consistency, replace with fail(err instanceof Error ? err.message : String(err)).

_{ccr:fp=7750c16137ef}

ccr:label=minor — 重复的一致性 finding；fork catch 已统一改用 fail() #stale

qiankunli · 2026-07-30T12:39:55Z

@@ -157,9 +223,18 @@ async function run(command: string): Promise<void> {
      }


🤖 devloop code-review · seed-2.1-pro

This console.error + process.exit(1) pattern could also be replaced with fail() for consistency with the rest of the file. The fail() function (line 132-135) does exactly the same thing.

_{ccr:fp=d6ec7a1ebca5}

ccr:label=minor — 与另外两条相同的真实一致性问题；无 session 和 catch 两个分支都已收敛到 fail() #stale

qiankunli · 2026-07-30T12:39:56Z

+  const sessionId = reference.slice(separator + 1);
+  if (!sessionId) throw new Error(`native session reference has no id: ${reference}`);
+  return { harness, sessionId };
+}


🤖 devloop code-review · seed-2.1-pro

qualifiedReference has inconsistent error handling: it returns null when no : separator is found (line 61), but throws an Error when sessionId is empty after the separator (line 64). The caller resolveNativeSession only checks for null (line 104), not for thrown exceptions. Passing a malformed reference like "cx:" would cause an unhandled exception to bubble up. Consider returning null consistently for all invalid formats, or wrapping the call in a try/catch.

_{ccr:fp=cb8addd3c538}

ccr:label=wrong — 无冒号表示受支持的裸 ID，cx: 则是缺失 ID 的非法限定引用；抛错会由 resume/fork 的既有错误边界接住，语义并不矛盾 #textbook

qiankunli · 2026-07-30T12:39:58Z

+  );
+  const matches = inspected.flatMap((result) =>
+    result.status === "fulfilled" && result.value ? [result.value] : []
+  );


🤖 devloop code-review · seed-2.1-pro

In resolveNativeSession, when Promise.allSettled is used to probe all providers, rejection reasons from individual providers are silently swallowed when at least one provider succeeds. If a harness provider throws a genuine operational error (e.g., network failure, permission issue) rather than a simple "not found", the error is hidden. Consider logging rejected provider results even when a match is found, so operational issues don't go unnoticed.

_{ccr:fp=916a7a84917f}

ccr:label=important — 裸 ID 自动识别必须证明唯一性；现已在任一 Provider 失败且另一方命中时 fail closed，并要求用 cx: 或 cc: 消歧

qiankunli · 2026-07-30T12:39:59Z

+        payload: { state: "idle", stopReason: "end_turn" },
+      });
+      syncedSeq = session.summarizeTurnEvent(turnId).seq;
+    }


🤖 devloop code-review · seed-2.1-turbo

If a turn in the source has both userText and agentText as falsy (e.g., empty strings from transcript parsing), no events are appended for that turnId before the state_update. While summarizeTurnEvent won't throw (it finds the state_update), it will produce a turn summary with no userText, agentText, or toolCalls — which is semantically empty and could confuse downstream consumers. Consider either skipping turns with no content, or at minimum guarding against this edge case.

_{ccr:fp=0884ef789fe4}

ccr:label=wrong — Codex/Claude transcript 归一器只产出非空文本 entry，nativeSessionTurns 由每个 entry 建立至少一个字段，所述空 turn 在实际导入链路不可达 #cross-file

qiankunli · 2026-07-30T12:40:00Z

+        harness: source.harness,
+        nativeSessionId: source.nativeSessionId,
+      },
+    });


🤖 devloop code-review · seed-2.1-turbo

updateMeta (which persists nativeSessionOrigin) and setHarnessSession (which persists the harness session binding with harnessSessionId) are called in separate steps. If the process crashes between these two writes, the session will have nativeSessionOrigin but no harnessSessionId binding. Since findByNativeSession matches on harnessSessionId, a subsequent adoption of the same native session would create a duplicate Baton session, violating the single-owner invariant. Consider persisting the harness session binding atomically with the native session origin, or using the nativeSessionOrigin as a fallback in findByNativeSession.

_{ccr:fp=86c8872f7268}

ccr:label=important — 已把 nativeSessionOrigin 与 harnessSessions binding 合并到同一次 atomic meta replacement，避免崩溃留下半个 owner

qiankunli · 2026-07-30T12:40:01Z

+  child.stdout.setEncoding("utf8");
+  child.stdout.on("data", (chunk: string) => peer.feed(chunk));
+  // 必须持续消费 stderr，否则 app-server 写满 pipe 后会反压阻塞只读请求。
+  child.stderr.resume();


🤖 devloop code-review · seed-2.1-pro

stderr from the Codex app-server process is silently discarded via child.stderr.resume(). The adapter.ts (line 671-683) captures stderr and logs it via the log sink. Here, any startup failures, crashes, or diagnostics written to stderr are lost, making debugging extremely difficult. Additionally, the JsonRpcPeer is constructed without a log sink (line 127), so even JSON-RPC parse errors from stdout would be silently swallowed.

Suggestion: capture stderr and pass a log sink to JsonRpcPeer, consistent with the pattern in adapter.ts.

_{ccr:fp=75ad960a53aa}

ccr:label=minor — 物化前尚无 BatonSession 日志可写；现改为持续消费并保留 4 KiB stderr 尾部，在初始化或查找失败时随错误返回

qiankunli · 2026-07-30T12:40:03Z

+  // 必须持续消费 stderr，否则 app-server 写满 pipe 后会反压阻塞只读请求。
+  child.stderr.resume();
+  child.once("error", (error) => peer.close(`codex app-server spawn error: ${error.message}`));
+  child.once("close", (code) => peer.close(`codex app-server exited (${code})`));


🤖 devloop code-review · seed-2.1-pro

The close event handler unconditionally calls peer.close() with an error message even when the process exits cleanly (code 0). In adapter.ts (line 688-699), the close handler only logs an error when code !== 0. While peer.close() is guarded against double-close, the unconditional error message is misleading and inconsistent.

_{ccr:fp=efdbac081d46}

ccr:label=wrong — process close 无论 code 都必须关闭 transport 并拒绝尚未完成的 RPC；正常操作完成后的 close 是幂等空操作，而响应前 clean exit 仍是失败 #textbook

qiankunli · 2026-07-30T12:40:04Z

+      );
+    }
+    return matches[0];
+  }


🤖 devloop code-review · seed-2.1-pro

TOCTOU race: findByNativeSession and createFromNativeSession are not atomic. Two concurrent processes could both pass the findByNativeSession check (no existing match) and both call createFromNativeSession, creating duplicate Baton sessions for the same native session. The session lock can't prevent this because the session doesn't exist yet. Consider using file-level locking (e.g., a lock file in the project directory) or accepting that the multi-match guard in findByNativeSession will detect and error on the next lookup.

_{ccr:fp=19540ffd3fd8}

ccr:label=important — 已将查重与创建放入 store 内同一 O_EXCL 跨进程锁临界区，两个进程不能再同时通过检查并各自创建 owner

qiankunli · 2026-07-30T12:40:06Z

🤖 devloop code-review · origin/main..HEAD · 18e68c379 · models: deepseek-v4-pro×25, seed-2.1-pro×26, seed-2.1-turbo×25 · cost: 219s · ccr v1.9.0

11 finding(s)（11 条已作为独立 review thread 发布）

qiankunli · 2026-07-30T12:45:29Z

🤖 devloop code-review · seed-2.1-turbo

Prior review finding (cc1d839) still present: The fork catch block uses console.error(...) + process.exit(1) instead of fail(). Both achieve the same result, but fail() is the established error-handling pattern used throughout this file (resume, logs, plugins, etc.). For consistency, replace lines 238-241 with fail(err instanceof Error ? err.message : String(err));.

_{ccr:fp=3f50a7ba3520}

ccr:label=minor — 重复 finding；当前工作树已将 fork catch 与无 source 分支统一为 fail()，待本轮提交后消失 #stale

qiankunli · 2026-07-30T12:45:30Z

+}
+
+export const claudeNativeSessions: NativeSessionProvider = {
+  async inspect(sessionId): Promise<NativeSessionInfo | null> {


🤖 devloop code-review · deepseek-v4-pro

The inspect function signature is missing the options parameter required by the NativeSessionProvider interface. The interface defines inspect(sessionId: string, options: NativeSessionProviderOptions): Promise<NativeSessionInfo | null>, but this implementation only accepts sessionId. This compiles because TypeScript allows extra properties in object literals satisfying interfaces, but it means the options (config, cwd) are silently ignored, and the signature is technically incorrect. Compare with codexNativeSessions.inspect which accepts both parameters.

_{ccr:fp=c3bd0fbbb1c8}

ccr:label=wrong — TypeScript 允许实现省略未使用的尾部参数，严格 typecheck 已通过；强加 options 参数和类型导入不会改变契约或行为 #textbook

qiankunli · 2026-07-30T12:45:32Z

+    if (!info) return null;
+    const messages = await getSessionMessages(sessionId, {
+      ...(info.cwd ? { dir: info.cwd } : {}),
+    });


🤖 devloop code-review · deepseek-v4-pro

The inspect method does not handle errors from getSessionMessages. If getSessionMessages throws (e.g., network failure, permission error), the rejection propagates unchecked. While the resolution layer uses Promise.allSettled to capture rejections, the error message will be raw and uncurated. The Codex provider (inspectCodexSession) wraps its read call in try/catch and checks for missingThread to decide when to return null vs. rethrow. Consider adding a similar try/catch here to distinguish between "session not found" (return null) and genuine errors (rethrow or wrap).

_{ccr:fp=aeeeae4895cd}

ccr:label=wrong — SDK 异常应作为 operational failure 向 resolution/CLI 边界传播，不能在 Provider 中吞掉或伪装成 not-found；当前边界已分类并带出原始原因 #cross-file

qiankunli · 2026-07-30T12:45:33Z

+      target,
+      harness: definition.sessionKey,
+      provider: definition.nativeSessions,
+    }];


🤖 devloop code-review · deepseek-v4-pro

harness: definition.sessionKey uses the session key (e.g., "claude-code") instead of the canonical HarnessName ("claude"). This flows through ResolvedNativeSession.harness into materializeNativeSession → createFromNativeSession, where it's stored as the harness field in event payloads. While typed as string, this creates an inconsistency: match.target.harness is "claude" (from defaultHarnessTarget) but match.harness is "claude-code". Any code that later parses or filters the event harness field would need to handle both values. Consider using definition.id (the canonical HarnessName) for consistency with HarnessTarget.harness.

_{ccr:fp=a7cfeb5945c7}

ccr:label=wrong — registry 明确定义 sessionKey 是事件 harness 与 HarnessSessionMeta.harness 的稳定 wire key；canonical claude 属于 HarnessTarget.harness，二者不能混用 #cross-file

qiankunli · 2026-07-30T12:45:34Z

+      : []
+  );
+  const detail = failures.length ? `; lookup errors: ${failures.join("; ")}` : "";
+  throw new Error(`native session not found: ${reference}${detail}`);


🤖 devloop code-review · deepseek-v4-pro

Promise.allSettled catches all errors from inspectSource and collapses them into a single "not found" error message. Infrastructure failures (e.g., spawn errors, network issues, JSON-RPC errors) are indistinguishable from genuine "session not found" results. A user seeing "native session not found: abc; lookup errors: codex app-server exited (1)" would be confused about whether the session exists or the tool is broken. Consider distinguishing between "not found" (null returns) and genuine errors (rejected promises), and surfacing infrastructure errors separately instead of burying them in the detail string.

_{ccr:fp=b7edcf532735}

ccr:label=important — 已把零命中且有 Provider rejection 的结果改为 native session lookup failed，只有全部 Provider 正常返回 null 才报告 not found

qiankunli · 2026-07-30T12:45:36Z

+  child.stdout.setEncoding("utf8");
+  child.stdout.on("data", (chunk: string) => peer.feed(chunk));
+  // 必须持续消费 stderr，否则 app-server 写满 pipe 后会反压阻塞只读请求。
+  child.stderr.resume();


🤖 devloop code-review · deepseek-v4-pro

The child process stderr is only consumed via resume() to prevent backpressure, but its content is discarded entirely. The adapter.ts counterpart (line 671-683) captures and logs stderr output, which is essential for diagnosing app-server startup failures or runtime errors. Without this, troubleshooting inspect failures becomes significantly harder — the app-server may fail silently. Consider piping stderr to a logging sink similar to the adapter pattern.

_{ccr:fp=7906799e8d1b}

ccr:label=minor — 当前工作树已持续消费 stderr、保留 4 KiB 尾部，并在 Codex 初始化或查询失败时附到错误中 #stale

qiankunli · 2026-07-30T12:45:37Z

+    await peer.request(
+      "initialize",
+      {
+        clientInfo: { name: "baton", version: "0.0.1", title: "baton" },


🤖 devloop code-review · deepseek-v4-pro

Hardcoded client version "0.0.1" in the JSON-RPC initialize request. The actual version is "0.2.0" (package.json) / "0.2.11" (VERSION). While this is consistent with the same hardcoded value in adapter.ts:722, it means the Codex app-server receives a stale version string that doesn't reflect the actual tool version, potentially affecting feature negotiation or debugging. Consider importing package.json version (as bin.ts does) or using a shared constant.

_{ccr:fp=24e4036a4ea7}

ccr:label=debatable — clientInfo version 确实是静态值，但 adapter 既有契约也使用同值，且 package.json 与内部 VERSION 本身不同；在缺少明确协议语义和单一版本源时不扩大本次 session 导入改动 #pre-existing

qiankunli · 2026-07-30T12:45:38Z

🤖 devloop code-review · origin/main..HEAD · fbec06331 · models: deepseek-v4-pro×37, seed-2.1-pro×40, seed-2.1-turbo×34 · cost: 509s · ccr v1.9.0

7 finding(s)（7 条已作为独立 review thread 发布）

Serialize native-session ownership checks and creation across processes, and persist provenance with the Harness binding in one atomic metadata replacement. Fail closed when bare-ID provider discovery is incomplete, distinguish lookup failures from genuine absence, retain Codex stderr diagnostics, and align fork errors with the shared CLI failure path.

qiankunli · 2026-07-30T12:54:05Z

+        process.argv.push("--session", opened.session.id);
+      } catch (err) {
+        fail(err instanceof Error ? err.message : String(err));
+      }


🤖 devloop code-review · seed-2.1-turbo

The native session import logic (resolveNative + materializeNativeSession) is duplicated between the resume case (lines 194-201) and fork case (lines 226-232). The only difference is the log message prefix. Consider extracting a shared helper to reduce duplication and ensure consistent error handling.

_{ccr:fp=50b0ce185fd8}

qiankunli · 2026-07-30T12:54:06Z


+  private acquireNativeSessionLock(): () => void {
+    mkdirSync(this.rootDir, { recursive: true });
+    const path = join(this.rootDir, "native-session.lock");


🤖 devloop code-review · seed-2.1-pro

The native session lock is placed at rootDir level (~/.baton/native-session.lock), which is shared across all projects. This means entirely unrelated materializations in different project directories will serialize on the same lock, causing unnecessary contention. Additionally, the materializeNativeSession method acquires the lock but findByNativeSession (called inside the lock) scans across all projects via listSessions(). If the goal is to serialize only the check-and-create for a specific native session ID, the lock should be scoped to the project or use a per-session lock file (e.g., join(this.rootDir, 'native-session-' + hash(source.nativeSessionId) + '.lock')).

_{ccr:fp=1aed2b606ea5}

qiankunli · 2026-07-30T12:54:08Z

+          console.log(
+            `${imported.reused ? "using imported" : "imported"} ${match.target.harness} native session ${match.source.nativeSessionId} as ${batonSourceId}`,
+          );
+        }


🤖 devloop code-review · deepseek-v4-pro

In the fork case, when sourceId is a non-bs_ prefixed ID and the native session import fails (e.g., the native session no longer exists), the error message from resolveNative will say "native session not found" — but the user may have been trying to fork a BatonSession by ID (not a native session). The fallback to store.listSessions({ cwd })[0]?.batonSessionId at line 219 will never be reached for non-bs_ IDs because the code treats them as native references. Consider distinguishing between BatonSession IDs and native session references more clearly, or providing a more helpful error message that suggests the user may have a typo in their BatonSession ID.

_{ccr:fp=1c0221826e39}

qiankunli · 2026-07-30T12:54:09Z

+    for (let attempt = 0; attempt < 3; attempt++) {
+      try {
+        const fd = openSync(path, "wx");
+        writeSync(fd, String(process.pid));


🤖 devloop code-review · seed-2.1-turbo

The lock token uses only the PID (String(process.pid)) without a timestamp or random component. On Unix, PIDs can be reused after a process dies. If the original process holding the lock dies and a new unrelated process gets the same PID, pidAlive(holder) will return true for the new process, causing the lock to appear valid when it is actually stale. This could lead to permanent materialization failures until the reused PID dies again. Use a token format like ${process.pid}:${Date.now()}:${Math.random()} as done in file-lock.ts to prevent this.

_{ccr:fp=28588781b9f8}

qiankunli · 2026-07-30T12:54:10Z

+
+function missingThread(error: unknown): boolean {
+  const message = error instanceof Error ? error.message : String(error);
+  return /thread.*not found|no rollout found|session.*not found/i.test(message);


🤖 devloop code-review · deepseek-v4-pro

The missingThread regex /thread.*not found|no rollout found|session.*not found/i is quite broad. The .* between thread and not found could match unrelated error messages (e.g., "thread permission denied: action not found"). Consider using more specific patterns or matching against known Codex error codes if available, to avoid masking genuine errors as "missing thread".

_{ccr:fp=ca96e54ecf52}

qiankunli · 2026-07-30T12:54:12Z

+  let stderr = "";
+  child.stderr.setEncoding("utf8");
+  child.stderr.on("data", (chunk: string) => {
+    // 物化前还没有 BatonSession 日志；保留有界尾部，在发现失败时带回诊断。


🤖 devloop code-review · deepseek-v4-pro

The comment on line 133 uses Chinese characters. While the codebase has some Chinese comments elsewhere (as seen in native-session.ts and jsonrpc.ts), this is inconsistent with most of the codebase which uses English. Consider using English for consistency.

_{ccr:fp=ef6eef9dabd7}

qiankunli · 2026-07-30T12:54:13Z

+  private acquireNativeSessionLock(): () => void {
+    mkdirSync(this.rootDir, { recursive: true });
+    const path = join(this.rootDir, "native-session.lock");
+    for (let attempt = 0; attempt < 3; attempt++) {


🤖 devloop code-review · seed-2.1-pro

The lock acquisition loop retries up to 3 times with no backoff delay between attempts. If two processes race, all 3 attempts could complete in under a millisecond, well before the lock holder releases it. This makes the retry essentially useless. Add a small delay (e.g., 10ms via Atomics.wait or setTimeout) between retries to give the holder time to release.

_{ccr:fp=7f99c5b84f4f}

qiankunli · 2026-07-30T12:54:14Z

+      try {
+        const store = new SessionStore(root);
+        const match = await resolveNative(id, { root, cwd });
+        const opened = materializeNativeSession(store, match, { cwd });


🤖 devloop code-review · deepseek-v4-pro

In the resume case, the TTY check at line 193 (!process.stdout.isTTY) is placed before the try block. This means the TTY check is not inside the try/catch, so if fail() is called here, it will exit with process.exit(1) directly (which is correct for fail). However, the resolveNative call at line 196 may also fail if stdin is not a TTY and there are multiple matches — but by that point we're inside the try block and the error will be caught. This is fine, but note that the resolveNative at line 196 has choose only when both stdin and stdout are TTYs, while the guard at line 193 only checks stdout. If stdin is not a TTY but stdout is, the guard passes but resolveNative won't have a choose callback, which will produce a clear error message from resolveNativeSession. This behavior is consistent.

_{ccr:fp=13dfa45c6dac}

qiankunli · 2026-07-30T12:54:15Z

+      } catch {
+        continue;
+      }
+      if (Number.isFinite(holder) && holder > 0 && pidAlive(holder)) {


🤖 devloop code-review · deepseek-v4-pro

If the lock file contains non-numeric content (corruption, partial write), Number(readFileSync(...)) returns NaN, which fails the Number.isFinite(holder) && holder > 0 check, silently deleting the lock and allowing concurrent acquisition. The lock file should be validated more strictly: verify it contains only a valid PID format before treating it as stale. Consider using the same removeStaleLock approach from file-lock.ts which re-reads the file before deletion to avoid TOCTOU races.

_{ccr:fp=17a1e12c779b}

qiankunli · 2026-07-30T12:54:17Z

+    throw new Error(
+      `native session lookup incomplete for ${reference}: ${failures.join("; ")}; use cx: or cc:`,
+    );
+  }


🤖 devloop code-review · seed-2.1-pro

The error messages on lines 129 and 140 hardcode "cx: or cc:" as harness disambiguation hints. These prefixes are derived from HARNESS_IDENTITIES aliases. If a new harness is added to the registry, these messages will become stale and misleading. Consider deriving the hint dynamically from the registered harness aliases (e.g., from HARNESS_IDENTITIES).

_{ccr:fp=7688b87ccefc}

qiankunli · 2026-07-30T12:54:18Z

+    const names = matches.map((match) => match.target.harness).join(", ");
+    throw new Error(
+      `native session id is ambiguous (${names}): ${reference}; use cx: or cc:`,
+    );


🤖 devloop code-review · seed-2.1-pro

Same hardcoded "cx: or cc:" disambiguation hint as noted above. Should be derived from registered harness aliases to stay consistent when harnesses are added or removed.

_{ccr:fp=dbc2c7a520df}

qiankunli · 2026-07-30T12:54:19Z

+  const separator = reference.indexOf(":");
+  if (separator < 0) return null;
+  const harness = reference.slice(0, separator);
+  const sessionId = reference.slice(separator + 1);


🤖 devloop code-review · seed-2.1-pro

qualifiedReference uses reference.indexOf(":") to split harness prefix from session ID. If a session ID ever contains a colon (e.g., certain UUID formats with colons, or future ID schemes), the split will be incorrect — the session ID will be truncated at the first colon. Consider using a more robust separator, or validate that the substring before the first colon is a known harness name/alias before splitting.

_{ccr:fp=e4c97b19db31}

qiankunli · 2026-07-30T12:54:20Z

    return explicitTitle ?? meta.description?.trim() ?? `fork: chat @ ${meta.cwd}`;
  }
-  return explicitTitle ?? meta.preview?.trim() ?? `chat @ ${meta.cwd}`;
+  return explicitTitle ?? meta.preview?.trim() ?? meta.description?.trim() ?? `chat @ ${meta.cwd}`;


🤖 devloop code-review · seed-2.1-turbo

The sessionDisplayTitle function now falls through to meta.description?.trim() as a final fallback. For native sessions, description is set to "import: ${source.harness} ${label}" which is a machine-readable provenance string, not a user-friendly title. Users will see raw import strings like "import: codex abc123" in the session picker if no preview is available. Consider filtering out the "import:" prefix or providing a more readable fallback for native sessions.

_{ccr:fp=291f7db4621b}

qiankunli · 2026-07-30T12:54:22Z

+
+  private createFromNativeSession(source: NativeSessionMaterialization): SessionHandle {
+    const session = this.createSession({ cwd: source.cwd, title: source.title });
+    const label = source.title?.trim() || source.nativeSessionId;


🤖 devloop code-review · seed-2.1-turbo

Variable label is already const, not let. The prior review's observation about this was incorrect — no issue here.

_{ccr:fp=bd6121906810}

qiankunli · 2026-07-30T12:54:23Z

+}
+
+export const claudeNativeSessions: NativeSessionProvider = {
+  async inspect(sessionId): Promise<NativeSessionInfo | null> {


🤖 devloop code-review · deepseek-v4-pro

The inspect method signature omits the options parameter that the NativeSessionProvider interface declares. While TypeScript allows a narrower parameter list, the Codex counterpart (codexNativeSessions) uses options.config.codexCommand and options.cwd — readers will expect the same shape here. If the Claude SDK ever needs a config-driven path or the caller starts passing structured options, this silent omission could become a bug.

Add options: NativeSessionProviderOptions as the second parameter (even if unused) to explicitly align with the interface contract.

_{ccr:fp=cec59f60bd42}

qiankunli · 2026-07-30T12:54:24Z

🤖 devloop code-review · origin/main..HEAD · 00f349604 · models: deepseek-v4-pro×29, seed-2.1-pro×29, seed-2.1-turbo×29 · cost: 237s · ccr v1.9.0

15 finding(s)（15 条已作为独立 review thread 发布）

qiankunli commented Jul 30, 2026

View reviewed changes

liqiankun1111 added 2 commits July 30, 2026 20:35

docs: align native session terminology

fbec063

qiankunli commented Jul 30, 2026

View reviewed changes

qiankunli merged commit ece9a4f into main Jul 30, 2026

		@@ -157,9 +223,18 @@ async function run(command: string): Promise<void> {
		}

Uh oh!

Conversation

qiankunli commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiankunli commented Jul 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiankunli commented Jul 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

qiankunli commented Jul 30, 2026 •

edited

Loading