test: cover Linear webhook delivery dedupe#827
Conversation
📝 WalkthroughWalkthroughAdds a new Vitest test file ( ChangesPOST /webhook Vitest Test Suite
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
![open-inspect[bot]](https://avatars.githubusercontent.com/u/2492022?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
PR #827 (test: cover Linear webhook delivery dedupe) by @ColeMurray adds route-level Linear webhook tests for delivery-header-based deduplication and malformed payload rejection. The coverage is focused and aligns with the current webhook behavior.
Files changed: 1 file, +197/-0.
Critical Issues
None found.
Suggestions
None.
Nitpicks
None.
Positive Feedback
- Covers the key regression risk that
webhookIdis a stable config identifier and should not be used as the dedupe key. - Verifies validation ordering by asserting malformed/missing-delivery cases do not touch KV or enqueue work.
- Uses a mocked handler at the route boundary, keeping these tests focused on request validation, dedupe, and dispatch behavior.
Questions
None.
Verdict
Approve.
Testing: npm test -w @open-inspect/linear-bot passed with 8 test files and 109 tests.
| .join(""); | ||
| } | ||
|
|
||
| function createFakeKV(initial: Record<string, string> = {}) { |
There was a problem hiding this comment.
[deep review] This adds a second bespoke fake KV implementation, plus another local env/context harness below, rather than creating one canonical Linear worker test harness. For a route-level test file this is exactly where the test infrastructure will keep growing: every future webhook route case will need signed requests, a fake KV with call inspection, a full Env, and an ExecutionContext. Can we extract the reusable pieces now, for example a small test-helpers module with createFakeKV, makeLinearBotEnv, makeExecutionContext, and signLinearWebhookRequest, and have both this file and kv-store.test.ts use it where applicable? That deletes duplicated scaffolding instead of normalizing another local copy with casts and hand-rolled behavior.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/linear-bot/src/index.test.ts (1)
178-196: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert handler non-invocation explicitly in malformed-payload test.
Add an explicit assertion that the mocked handler is never called on malformed payloads to lock the contract more tightly.
Suggested patch
expect(res.status).toBe(400); expect(await res.json()).toEqual({ error: "Invalid payload" }); expect(kv.get).not.toHaveBeenCalled(); expect(kv.put).not.toHaveBeenCalled(); expect(ctx.waitUntil).not.toHaveBeenCalled(); + expect(mocks.handleAgentSessionEvent).not.toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/linear-bot/src/index.test.ts` around lines 178 - 196, In the malformed AgentSessionEvent test in index.test.ts, explicitly assert that the webhook handler is never invoked when validation fails, not just that KV and waitUntil are skipped. Add a clear non-invocation expectation against the mocked handler used by app.fetch for this payload, alongside the existing Invalid payload and 400 assertions, to lock the contract in the malformed-payload path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/linear-bot/src/index.test.ts`:
- Around line 178-196: In the malformed AgentSessionEvent test in index.test.ts,
explicitly assert that the webhook handler is never invoked when validation
fails, not just that KV and waitUntil are skipped. Add a clear non-invocation
expectation against the mocked handler used by app.fetch for this payload,
alongside the existing Invalid payload and 400 assertions, to lock the contract
in the malformed-payload path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c51fc6c-1ba1-4458-b425-de99fcd29531
📒 Files selected for processing (1)
packages/linear-bot/src/index.test.ts
1 participant
Summary
Linear-Deliverydedupe contract.webhookId, and malformed AgentSession payload rejection.Why
These paths decide whether external Linear webhook deliveries enqueue agent work, so regressions can silently drop or duplicate sessions.
Testing
npm test -w @open-inspect/linear-botCreated with Open-Inspect
Summary by CodeRabbit