Handoff reliability + read_output screen view (plan 3712ddc) - #42
Merged
Conversation
- handoff: groupManagerApi の getGroup 欠落 (repo_info 失敗の確定原因) と、 takeHandoff のウェイターが接続死活を知らないことによるイベント喪失レースを修正する計画。 - read_output: 生バイト連結では復元できない TUI 差分描画を、軽量仮想画面モデルと 画面変化ベースの idle 時間 (screen / screenIdleMs) で観測できるようにする計画。
…2ddc) handoff (問題1): - groupManagerApi に getGroup を追加 (repo_info の TypeError 確定原因の修正)。 facade とツールの対応をコメントで明示し、テストは実 facade (getGroupManagerApi) 経由に変更 -- 本番/テストの deps 形状ズレの再発を防止。 - takeHandoff に opts.isAlive (接続死活) を追加: 死んだ接続のウェイターは イベントを dequeue しない。mcpBroker が接続ごとの死活クロージャを注入 (SocketTransport 生成後に buildServer(identity, connectionIsAlive))。 - supersede 時に stale が消費済みのイベントをキューの先頭へ戻す再キュー化 (参照一致で二重 re-queue 防止)、stale への返値を {orphaned:true} から {timedOut:true} へ統一。onOrchestratorExit で pendingTakes を settle (15 分のゾンビウェイターを残さない。イベントはキューに残る)。 - テスト: facade 経由 deps 化、supersede 再キュー / isAlive 非消費 / onOrchestratorExit settle、wire レベルの接続死後イベント到達 + repo_info (isError なし) を追加。 read_output (問題2): - 軽量仮想画面モデル server/ws/screenModel.js を新設 (ANSI サブセット解釈、 200x80 有界、chunk 跨ぎエスケープ保持、TextDecoder stream で UTF-8 跨ぎ対応)。 - sessionManager が出力 chunk ごとに screen を並行維持し、 screenLastChangeAt (画面変化ベース) を更新。read_output に screen / screenAlt / screenTruncated / screenIdleMs を追加、get_tab_status に screenIdleMs を追加 (idleForMs は互換のため残す)。 - text の 16KB キャップをエスケープシーケンスを割らない境界でカット (plan の chunk 粒度案より厳密: 途中切断による制御文字漏れを除去)。 - テンプレート/ツール説明/README を新 API と handoff 保証に更新。 検証: npm test 318 tests / 317 pass / 0 fail (1 skipped)、 npm run build --workspace=client 成功。実シェルセッションで screen が スピナー最終フレームを表示し screenIdleMs が増加することを確認。 E2E (playwright) はこの環境でブラウザバイナリが取得できず起動不可 (MS CDN ゲートウェイエラー、全失敗がブラウザ起動エラー) のため未実施。
cleanTextCut cut at a sequence-safe boundary, but a stream that ends mid-sequence (a pty chunk boundary split the last escape) still leaked the bare partial sequence through stripAnsi into the text view. Trim the dangling escape from the tail in both the capped and uncapped paths; regression tests cover both.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the two plans in
tmp/:handoff_to_orchestratorandwait_for_handoff.takeHandoffis now connection-aware: an event is only dequeued when the waiting connection is alive (isAliveguard), a superseded waiter reclaims any event it had already claimed (re-queue instead of loss), superseded/stale waiters settle as{timedOut:true}(the documented shape, not the old undocumented{orphaned:true}), andonOrchestratorExitsettles pending waiters instead of leaving 15-minute zombies.connectionIsAlive), threaded intowaitForHandoff.repo_info: the production broker facade previously lackedgetGroup, making repo_info fail in production with "deps.groupManager.getGroup is not a function" while full-module unit tests stayed green. Since then master (PR Fix copilot --mcp-config injection and repo_info facade mismatch #39) fixed this with a better design (getGroupCwd-- cwd only, no group internals exposed); this branch merged master and adopted that design, keeping the facade-completeness test seams (getGroupManagerApi+ wire-level repo_info test) so the mismatch class cannot recur.screenModel.js).read_outputgainsscreen(current visible rows),screenAlt,screenTruncated,screenIdleMs(ms since the screen last visibly changed -- the reliable busy/idle signal);get_tab_statusgainsscreenIdleMs.textis now cut at a boundary that never splits an escape sequence (no bare control bytes leak past the 16KB cap), and dangling escapes at the end of the stream are trimmed.Includes a merge of the current master (BUN_TMPDIR fix, notify attribution/discipline).