mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
DottaandClaude Opus 5.5
223687bbb9
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Chat channels connect GitHub, Slack, and other providers to tasks,
and `chat-channels.integration.test.ts` covers them against a real
embedded Postgres
> - Two parameterized tests do much more database work than the other
tests in the file, but they use the global 15s `testTimeout`
> - On a slow CI runner they time out. Vitest then keeps the timed-out
test body running, and that body breaks later, unrelated tests in the
same file
> - This failure blocks PRs that do not touch chat code
> - This pull request gives the two heavy tests a time budget that fits
their workload
> - The benefit is that a slow runner no longer turns one heavy test
into a cascade of 2–7 failures
## Linked Issues or Issue Description
**What happened?**
In run 42098299218 (job 127495756270, shard `General tests (chat
(3/3))`), 7 chat tests failed in two Vitest processes on a PR that
changed only unrelated test files:
- Process 1: `reactivates denied retry notices when authority is
restored during bounded pass` timed out at 15000ms. Then `… during
requester merge` failed with `A function to advance timers was called
but the timers APIs are not mocked`.
- Process 2: `preserves one current GitHub Retry across backlog_51` and
`… consumed_backlog_52` timed out at 15000ms. About 110 tests later,
three pre-run FIFO tests failed: `advances pre-run FIFO selection past a
suppressed page with microsecond timestamps (manual)` (`expected +0 to
be 1`), `retires an already-visible pre-run FIFO notice once without
publishing scheduler failures` (`expected [] to have a length of 1`),
and `retries a contended pre-run FIFO receipt before I/O and observes
cancellation after the lock releases` (`expected undefined to match
object { state: 'retry' }`).
**Expected behavior**
These tests pass on a slower runner. A timeout in one test does not
change the result of a different test.
**Steps to reproduce**
1. On `master`, in `server/`, run:
`pnpm exec vitest run src/__tests__/chat-channels.integration.test.ts -t
"GitHub Retry across (backlog_51|consumed_backlog_52)|advances pre-run
FIFO selection past a suppressed page|retires an already-visible pre-run
FIFO notice once|retries a contended pre-run FIFO receipt before I/O"
--testTimeout=1200`
2. The lower timeout simulates a slow runner. The two backlog tests time
out, and then the same three FIFO tests fail with the same assertions as
in CI. The `scoped` FIFO case passes, as it did in CI.
3. Run the same command without `--testTimeout`. All 6 tests pass.
**Root cause**
- The workload is large. `backlog_51` and `consumed_backlog_52` deliver
51–52 real inbound GitHub messages through the full delivery path.
`bounded pass` inserts and authorizes 100 rejected retry notices.
- CI timing data supports this. On passing runs these tests take
5.4–7.7s (backlog) and 3.6–5.3s (bounded pass), and lighter tests in the
same file take 0.5–1s. On the failing runner, the lighter tests were
2–3x slower (for example, `idle` took 1856ms instead of 500–680ms). This
put the heavy tests over 15s.
- When Vitest times out a test, it does not stop the test body. The body
keeps running while the next tests start:
- The unfinished backlog tests left 51+ `deferred_issue_execution`
inbound wakeups. The pre-run FIFO tests use unscoped, bounded passes
(`enqueueInboundWakeupPublications`, `processPendingPublications`). The
leftover rows filled those pages, so the new test's row was not reached.
- The unfinished `bounded pass` body later ran its `finally` block,
which calls `vi.useRealTimers()`. This disabled the fake timers that
`requester merge` had just installed.
## What Changed
- `preserves one current GitHub Retry across %s`: set an explicit 60s
timeout.
- `reactivates denied retry notices when authority is restored during
%s`: set an explicit 60s timeout. Increase the `bounded pass` wait for
its held 100-row reconciliation from 10s to 30s, so that the inner wait
does not become the new limit.
- The same file already uses explicit 20s, 25s, 30s, 40s, 60s, and 90s
budgets for other heavy tests. No product code changed.
## Verification
- Red: the reproduction command above fails on `master` with the CI
cascade. A tighter `--testTimeout=1600` on `reactivates denied retry
notices` also shows that later modes fail after `bounded pass` times
out.
- Green: the same reproduction command with this change (still
`--testTimeout=1200` for all other tests) passes 10/10 selected tests,
including the three FIFO tests.
- `pnpm exec vitest run src/__tests__/chat-channels.integration.test.ts
-t "GitHub Retry across|reactivates denied retry notices|pre-run FIFO"`:
46 passed.
## Risks
- Low risk. This is a test-only change. A real hang in these two tests
now fails after 60s instead of 15s. This is well inside the job limit.
- The cascade can still occur if one of these tests needs more than 60s.
That would mean a runner about 10x slower than a normal one.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude
Code with tool use (shell, file edits, GitHub CLI). Default context
window. No extended thinking mode.
## 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
- [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 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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>