test: wait for Slack workers before checking settled queues (#16698)

## Thinking Path

> - Paperclip is an open source app that manages AI agents for work.
> - Slack delivery uses scheduled workers to process queued actions.
> - The queue test checks that a settled queue does not poll the
database.
> - The next scheduled wake can be empty while a worker still runs.
> - This pull request waits for the tracked workers to stop before that
check.
> - The test still rejects all queries during the 1200 ms quiet period.

## Linked Issues or Issue Description

**What happened?**

The test `wakes Slack session sync from publication commit and leaves
settled action queues quiet` can report three unexpected `select` calls.
The worker can clear its scheduled wake before its final deadline read
and empty queue scan end. The test starts its query spy in that
interval.

The failure was observed in [this CI
job](https://github.com/paperclipai/paperclip/actions/runs/38854774831/job/116823717452)
on [the SSH summary-slot pull
request](https://github.com/paperclipai/paperclip/pull/16535). This test
race also reproduces on `master` without the SSH change. This pull
request contains only the independent test fix.

**Expected behavior**

Start the quiet-period assertion after the scheduled work and the active
workers are both idle. Continue to fail if any query occurs during the
next 1200 ms.

**Steps to reproduce**

1. On the base commit below, run the target test. The failure depends on
timing.
2. To expose the interval, add this temporary control after the
coordinator is created in that test, before work is registered:

```ts
const readDeadline = f.service.nextChatActionAt.bind(f.service);
vi.spyOn(f.service, "nextChatActionAt").mockImplementation(async (kind) => {
  if (kind === "slack_session_sync") await new Promise(resolve => setTimeout(resolve, 150));
  return readDeadline(kind);
});
```

3. Run `pnpm exec vitest run
server/src/__tests__/chat-channels.integration.test.ts -t 'wakes Slack
session sync from publication commit and leaves settled action queues
quiet'`. The old wait fails with three `select` calls.
4. Apply this change and run the same control. The test passes with zero
queries during the quiet period.
5. Remove the temporary control. It is not part of this pull request.

**Paperclip version or commit**

Base: `4415665d67b16ffefbe4519c4bdd521f37bb87b9` on `master`.

**Deployment mode**

Local test run from source. The fixture uses embedded PostgreSQL and the
test Slack runtime. No live Slack workspace was used.

## What Changed

- Require both `coordinator.nextWakeAt() === null` and
`idleWorkSnapshot().active === 0` in the existing wait.
- Keep the query spy, 1200 ms observation period, and zero-query
assertion unchanged.
- Reuse the existing import and wait pattern. No production code,
dependencies, or configuration changed.

## Verification

- Controlled reproduction on the base: one target test failed with three
unexpected queries. With the same 150 ms control and this fix: one
target test passed. The control was then removed.
- `pnpm exec vitest run
server/src/__tests__/chat-channels.integration.test.ts -t 'returns two
answered Slack turns to Idle in the same thread|wakes Slack session sync
from publication commit and leaves settled action queues quiet|sleeps
behind an unresolved Slack tool send and wakes when its receipt
settles'`: three tests passed; 1344 tests were skipped by the name
filter. This run used the final source without the temporary control.
- `pnpm exec vitest run
server/src/__tests__/delivery-work-coordinator.test.ts
server/src/__tests__/work-scheduler.test.ts`: 20 tests passed in two
files.
- `git diff --check`: passed.

Environment: macOS ARM64, Node 26.12.0, pnpm 9.15.4, Vitest 5.0.3.
Existing dependencies were reused in an isolated worktree. The
shared/plugin SDK build passed.

The CI chat shard was attempted with `pnpm exec node
scripts/run-vitest-stable.mjs --mode general --group general-chat
--shard-index 2 --shard-count 3`. The runner verified its collected
filters, then reported `No test found ... in lines ...`. That invocation
skipped all 1347 tests. Its sibling invocation was stopped after this
runner failure. The shard did not pass. The first name-filtered retry
stopped in fixture setup with `ENOSPC`; task-owned scratch directories
were then removed before the final retry.

Full workspace typecheck, test, and build were not run. The isolated
environment does not have the full CLI/workspace dependencies or Cargo.
Linux with Node 24, new pull request CI, and Greptile were not run.

## Risks

Low risk. This changes a test wait only. It does not change delivery
behavior or database data. The existing global idle counter is valid for
this sequential fixture. If the suite later runs fixtures concurrently,
the wait may also wait for unrelated tracked work.

## Model Used

OpenAI Codex assisted with diagnosis, implementation, and validation.
The session configuration was `gpt-6.1-sol` with `xhigh` reasoning.
Read-only investigation and review used `gpt-6-luna` with `max`
reasoning. Browser preparation used `gpt-6.1-sol` with `medium`
reasoning. Tools included shell execution, Git, GitHub, Vitest, and
browser automation. The context window size was not exposed.

## 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 either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [ ] 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 any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
This commit is contained in:
Ethereal49 authored and GitHub committed 2026-10-29 15:58:27 -05:00
1 parent 35ab90ad0f
commit 649354d8c4
1 file changed
+4 -1
@@ -36853,7 +36853,10 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
await expect.poll(async () => db.select({ status: chatActions.status }).from(chatActions)
.where(and(eq(chatActions.endpointId, lane.endpoint.id), eq(chatActions.kind, "slack_session_sync"))),
{ timeout: 10_000 }).toEqual([{ status: "processed" }]);
await vi.waitFor(() => expect(coordinator.nextWakeAt()).toBeNull(), { timeout: 10_000 });
await vi.waitFor(() => {
expect(coordinator.nextWakeAt()).toBeNull();
expect(idleWorkSnapshot().active).toBe(0);
}, { timeout: 10_000 });
const scans = vi.spyOn(db, "select");
try {
await new Promise(resolve => setTimeout(resolve, 1200));