[lilyshen0722]

Why

agent-states-listener-check-in.test.js › "an MCP call alone is never-connected; a check-in on the User token flips it to listening" failed in two of the last five merge-queue runs on 11-04 (runs 41922149243 and 41923958639) with Expected: "listening" / Received: "never-connected", and dequeued unrelated PRs (#3446 twice).

The check-in write is fire-and-forget, and the test slept a fixed 150 ms before reading the route. That race is lost under queue load: it's the file's first test, running while #3443's new AgentEvent index builds. It is not a regression: it passes 3 of 3 on main locally, and main's push-run Backend tests are green, per SRE.

Change (test only)

The test now waits until the lastListenedAt stamp is in the database (Model.exists, polled every 25 ms, for up to 5 s), then reads the route once. Polling the route itself was tried first: it trips the route's rate limit (429).

Proof

• Green: 3 of 3 local runs pass.
• Reverse: with both recordListenerCheckIn calls removed, both tests still fail (Received: "never-connected"), so the wait can't hide a check-in that never writes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MDs9DPFVAooJnqCSwrdxVw

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The tests replace a fixed 150 ms delay with polling for lastListenedAt on the User or AgentInstallation record. Polling checks every 25 ms for up to five seconds.

Changes: Listener check-in tests

Layer / File(s)	Summary
Poll for check-in timestamps backend/__tests__/service/agent-states-listener-check-in.test.js	The helper polls for a matching record every 25 ms, up to five seconds. Both token tests wait for lastListenedAt to become non-null before asserting that the route reports listening.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related PRs

• Team-Commonly/commonly#3130: Adds the check-in behavior and test file that this PR updates.
• Team-Commonly/commonly#3114: Adds the helper that writes lastListenedAt, which these tests poll for.

Merge Risk: Minimal · up to d9b1a

This test change removes timing dependence while retaining failure detection for missing check-ins, so it is mergeable with normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the change accurately: the listener check-in test now waits for the database write instead of a fixed 150 ms delay. It is specific and matches the changeset. The title uses the convent…
Description check		The description is directly related to the changeset. It explains the flaky agent-states-listener-check-in test, the fixed 150 ms sleep race, and the fix that polls Model.exists every 25 ms for up to …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the change accurately: the listener check-in test now waits for the database write instead of a fixed 150 ms delay. It is specific and matches the changeset. The title uses the conventional test(agents): prefix, which is acceptable. Its length (80 characters) is at the upper end, but it remains a single clear sentence.

Full details: Description check

Explanation

The description is directly related to the changeset. It explains the flaky agent-states-listener-check-in test, the fixed 150 ms sleep race, and the fix that polls Model.exists every 25 ms for up to 5 s. It also covers the rejected route-polling approach (429), the local test runs, and the reverse check that removes both recordListenerCheckIn calls.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: Team-Commonly/commonly
• Branch: main
• File: .coderabbit.yaml
• Commit: 1b84804
• Layers: Repository YAML
• Review profile: QUIET
• Plan: Advanced
• Run ID: 13bdb25b-cda3-4783-afad-9360742c4ab1

Commits

Reviewing files that changed from the base of the PR and between 1b84804 and d9b1acf.

Files selected for processing: 1

• backend/__tests__/service/agent-states-listener-check-in.test.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

[lilyshen0722]

HEAD: d9b1acf — GATE: PASS

One quick review round (Tier 0, test only).

• The wait proves what the route reads. /agent-states → deriveAgentState → latestListen reads lastListenedAt straight from Mongo on AgentInstallation.runtimeTokens and on the bot User's agentRuntimeTokens, with no cache in between. Test 1 waits on the User field, which is the only source there, because its installation has no tokens. Test 2 waits on the installation field. $ne: null excludes both a missing field and a null one.
• The filters are specific. Each test filters on a name only it uses (mcp-only, install-only), and neither row can match the other's filter.
• A write that never lands still fails. landed returns after 5 s, then the route assertion fails with never-connected. That's under the 30 s testTimeout. The PR's reverse check confirms it.
• Flake risk is low. There's a single route read, so no rate limit; the throttle resets beforeEach; and each update is atomic.

GATE: PASS