Skip to content

test: control ACP disconnect fixture timing - #2002

Merged
zxch3n merged 1 commit into
mainfrom
fix/acp-batching-test-signal
Oct 28, 2026
Merged

zxch3n merged 1 commit into
mainfrom
fix/acp-batching-test-signal

Conversation

@zxch3n

@zxch3n zxch3n commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Related issue

Problem / pressure

The first CLI attempt of run 40630618794 timed out in the disconnected tool_call_update case; rerunning the job passed. The fixture froze timeout timers while loro-repo 0.25.0 measured persistence work with real performance.now(). A persistence yield could be captured by the fake clock, then discarded when cleanup switched clocks, stranding repo.destroy().

Summary

Keep one real timer clock for these repository lifetimes. Use explicit prompt/disconnect signals, synchronously cancel the scheduled batch flush, and exercise both buffered and explicitly persisted notifications before disconnecting. Preserve all existing assertions and the 30-second timeout; no retry or product-code changes. Add a bilingual testing note and correct the fixture clock comment.

Visual explanation

Before: persistence exceeds 5ms → fake setTimeout(0) → uninstall fake clock
                                                       ↓
                                        callback lost → repo.destroy waits

After:  notification → cancel batch timer → prompt-entered signal
        → leave buffered / await explicit flush → release disconnect
        → await execution, drain and repo close on the same timer clock

Before / after

Before After
Persistence cleanup depends on whether real work exceeds its budget while fake timers are installed. SDK persistence yields remain runnable through cleanup.
One implicit notification/disconnect order. Explicit gates cover buffered and persisted notifications for all three update types.

Test plan

  • Entire file: 20 consecutive independent Vitest processes, 74 cases each (1,480 passed), with GIT_EXEC_PATH cleared for synthetic Git fixtures.

  • Controlled slow branch: temporarily install a hoisted monotonic clock advancing 10ms per read before the SDK captures it. All six disconnect variants passed in five independent processes (30 cases); timer stacks confirmed DocManager.drainDirtyDocs yielded.

  • Ablation: remove only the real-timer fix under that injected clock. Execution, drain and assertions complete, then cleanup reproduces the original 30-second timeout.

  • The original CI log establishes a timeout, not its exact blocked await; the controlled reproduction establishes the fixture defect.

  • pnpm format, changed-file Oxfmt check, pnpm run docs check, workspace typechecking, both lint passes, i18n and all repository boundary guards passed.

  • pnpm check reached script tests, where the sandbox denied listen 127.0.0.1 (EPERM). Re-running pnpm test:ci with local-port access passed scripts and 6,405 component cases; CLI reported 5,006 passed, 4 skipped and one failure in the unchanged github-git-transport.test.ts recursive SSH fixture (context_unreadable). The changed batching file passed in that full CLI run. Direct Node execution with GIT_EXEC_PATH removed passed that Git fixture on both unmodified main 9fec3a21 and this head. Full pnpm check is therefore not claimed green; downstream package tests after the recursive-run failure did not finish.

Context handoff

Same-repository maintainer branch. Fixture-only change; no live-provider or packaged-desktop acceptance is claimed.

@zxch3n
zxch3n marked this pull request as ready for review October 28, 2026 05:11
@zxch3n
zxch3n merged commit af7ec64 into main Oct 28, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant