test(plugin-worker): remove the wall-clock race from the duplex buffered-replay tests (#12799)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - The plugin worker manager runs agent plugin workers through duplex
channels.
> - The duplex buffered-replay tests check data that arrives before a
listener attaches.
> - The tests used a fixed 60 ms sleep as the barrier for worker output.
> - Worker startup and output latency can exceed that delay under load.
> - This pull request uses a worker exit frame as a deterministic
barrier.
> - The benefit is stable test results without a product code change.
## Linked Issues or Issue Description
**What happened?**
The duplex buffered-replay tests used a fixed 60 ms sleep before they
attached a data listener. Under load, worker output could arrive after
the sleep. The tests then saw a partial buffer and failed.
**Expected behavior**
The tests must wait until the worker sends all three data frames before
they inspect the pre-bind buffer.
**Steps to reproduce**
1. Run npx vitest run src/__tests__/plugin-worker-manager-duplex.test.ts
in the server package.
2. Add a 200 ms or 800 ms delay to the worker fixture emit path.
3. Repeat the test run and observe the old fixed-sleep barrier fail
intermittently.
**Paperclip version or commit**
b773f0f2e2
**Deployment mode**
Built from source. This change affects tests only.
## What Changed
- Replace the fixed sleep in both buffered-replay tests with an
exit-frame barrier.
- Write the three data frames and the exit frame in one worker output
write.
- Wait for the session to settle before the tests attach listeners.
- Keep the non-batch buffer-then-drain path and the throwing-listener
behavior.
- Remove the retry wrapper from the first test because the drain runs
synchronously.
## Verification
- Run npx vitest run src/__tests__/plugin-worker-manager-duplex.test.ts
in the server package.
- The full file passes 35 of 35 tests.
- Run the full file 15 times. All 15 runs pass.
- Test the new barrier with 200 ms and 800 ms worker-output delays. Both
tests pass.
- The server type check still reports 71 pre-existing errors in
native-runtime and paperclip-runner. No new error appears in the changed
test file.
- Search GitHub for duplicate or related public issues and pull
requests. No duplicate open item exists.
- Check ROADMAP.md. This test-only fix does not duplicate planned core
work.
## Risks
- This change affects test synchronization only.
- The test could become invalid if the worker stops sending the exit
frame. The session wait then fails instead of hiding the problem behind
a clock delay.
- No product code, database schema, or runtime behavior changes.
## Model Used
OpenAI GPT-5, exact model ID gpt-5, API model with code execution and
tool use. The model used a 1M-token context window. No extended
reasoning mode was specified.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have described the issue in-PR following the bug report template
- [x] I have not referenced internal/instance-local Paperclip issues or
links
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
0ad180b85f
commit
333abdd2c2
|
|
@ -130,17 +130,22 @@ describe("plugin worker manager duplex channel route", () => {
|
|||
const session = await handle.openDuplexChannel(
|
||||
duplexOpenInput({
|
||||
data: [{ chunk: "one" }, { chunk: "two" }, { chunk: "three" }],
|
||||
exitCode: 0,
|
||||
}),
|
||||
);
|
||||
// Wait so the three data notifications arrive and buffer before a listener
|
||||
// attaches. The drain then delivers them in order.
|
||||
await new Promise((resolve) => setTimeout(resolve, 60));
|
||||
// The worker writes the three data notifications and the exit in one
|
||||
// stdout write. The host reads worker stdout line by line, so it buffers
|
||||
// all three data frames before it reads the exit. The exit settles the
|
||||
// wait, so the wait is a deterministic barrier: once it resolves, the
|
||||
// host holds all three frames and no listener has attached yet. This
|
||||
// barrier replaces a fixed sleep, so the test does not race the
|
||||
// subprocess start or the stdio latency.
|
||||
await session.wait();
|
||||
const chunks: string[] = [];
|
||||
// The session streams raw `Uint8Array` chunks. Decode each one back to
|
||||
// text, so the assertion below compares the plain-text payload the
|
||||
// fixture directive scripted.
|
||||
session.onData((chunk) => chunks.push(new TextDecoder().decode(chunk)));
|
||||
await vi.waitFor(() => expect(chunks.length).toBe(3));
|
||||
expect(chunks).toEqual(["one", "two", "three"]);
|
||||
await session.close();
|
||||
} finally {
|
||||
|
|
@ -213,11 +218,17 @@ describe("plugin worker manager duplex channel route", () => {
|
|||
const session = await handle.openDuplexChannel(
|
||||
duplexOpenInput({
|
||||
data: [{ chunk: "one" }, { chunk: "boom" }, { chunk: "three" }],
|
||||
exitCode: 0,
|
||||
}),
|
||||
);
|
||||
// Wait so the three data notifications arrive and buffer before a listener
|
||||
// attaches. The drain then delivers them in order.
|
||||
await new Promise((resolve) => setTimeout(resolve, 60));
|
||||
// The worker writes the three data notifications and the exit in one
|
||||
// stdout write. The host reads worker stdout line by line, so it buffers
|
||||
// all three data frames before it reads the exit. The exit settles the
|
||||
// wait, so the wait is a deterministic barrier for "the host holds every
|
||||
// pre-bind frame and no listener has attached". This barrier replaces a
|
||||
// fixed sleep, so the test does not race the subprocess start or the
|
||||
// stdio latency.
|
||||
await session.wait();
|
||||
const chunks: string[] = [];
|
||||
// The listener throws on one buffered chunk. The manager catches the throw
|
||||
// inside the drain, so it does not escape `onData` and every buffered chunk
|
||||
|
|
|
|||
Loading…
Reference in New Issue