[devswha]

Change

A signed-in device (a watch-only one included) could hold every one of the 16 read slots of
POST /api/connection-diagnostics by sending each report body a byte at a time: the read had a
size bound but no time bound, so every other report got 429 until those reads ended (#1425, item 1).

• server/client-connection-log.ts: each body now has one 10 s deadline for the whole read, not a
  per-chunk timeout. A timer covers a body that stops arriving; a clock check after each chunk covers
  one that keeps trickling. Past it the request gets 408 trace_timeout, the reader is cancelled,
  and the slot is released by the existing finally in handle().
• shared/protocol.ts: the route's comment lists the new 408.
• Item 4: src/lib/connectionTrace.test.ts closes every trace a test opens (a later test's
  resumeConnectionTraces() reached earlier tests' traces), and the new src/lib/api-signin.test.ts
  covers authenticate and pairDevice: request shape, reporting resumed after a successful sign-in,
  and not resumed after a refused one.

Items 2 and 3 are left as they are, as the issue describes them: the report quota is charged only
after validation so malformed posts cannot starve real ones (each parse is at most 8 KiB, 16 at once),
and a 4xx refusal drops that incident's batch by design (connectionTrace.ts, tested); the 408 follows
that rule and the next incident still posts.

This adds a status code to an HTTP route's contract, so it needs a real review.

Fixes #1425

Verification

• server/client-connection-log.test.ts: "ends a body that keeps trickling in once the deadline for
  the whole body passes" (fake clock, no sleeps) and "gives every read slot back when the bodies holding
  them stop arriving" (16 stalled bodies, 1 ms injected deadline). On origin/main they fail (413 / a 5 s
  test timeout); on this branch they pass.
• src/lib/api-signin.test.ts: removing resumeConnectionTraces() from api.ts fails the two
  "resume" cases; resuming before the ok check fails the two "refused" cases.
• Live, against createServer({ port: 0 }) with 16 raw-TCP senders at one byte per second, rotating
  X-Forwarded-For: this branch answers each HTTP/1.1 408 Request Timeout at 10.0 s and the next
  report gets 204; origin/main answers none of them within 20 s and the next report still gets 429.
• Reviews: omo-native-code-reviewer APPROVE (two LOW notes, both the documented trade-offs above);
  Codex (gpt-6.1-sol, xhigh) APPROVE with no findings.
• bun run check fast: workflow syntax, generated files, typecheck and build pass; unit tests
  6395 pass, 4 fail, all outside this change. Three in server/shell-agent.test.ts fail with EMFILE
  from fs.watch, on origin/main too: this PC is at its inotify instance limit (126 of 128) from
  other sessions. One in server/omp-process.test.ts is the readiness flake fix(tests): read omp-process test targets only once they are up #1862 and fix(omp): wait for process fixtures before reading procfs #1869 address;
  it passes when run alone.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: fix/connection-diagnostics-read-deadline
• File: .coderabbit.yaml
• Commit: f9667e5
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: cbd81606-228d-42f8-9637-21b36c5450f6

Commits

Reviewing files that changed from the base of the PR and between d3eefef and f9667e5.

Files selected for processing: 6

• changelog.d/connection-diagnostics-read-deadline.md
• server/client-connection-log.test.ts
• server/client-connection-log.ts
• shared/protocol.ts
• src/lib/api-signin.test.ts
• src/lib/connectionTrace.test.ts

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

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devswha]

CodeRabbit hit its review limit on this PR, and it changes an HTTP route's contract (a new 408), so here are the reviews that stand in for it (AGENTS.md):

omo-native-code-reviewer: APPROVE, two LOW notes:

1. Someone rotating X-Forwarded-For on loopback can still keep all 16 slots busy by reconnecting every 10 s. Left as is: this is item 2's accepted trade-off. The report quota is charged after validation so that malformed posts cannot starve real ones. Each such read is now bounded to 10 s, where before it was unbounded.
2. A complete body whose last chunk lands just after the 10 s deadline still gets 408. Left as is: that matches the documented "over 10 s to arrive" rule.

Codex (gpt-6.1-sol, xhigh reasoning): APPROVE, no findings. It ran the targeted tests and fixed-sleep checks, and confirmed that cancelled reads return their slots without unhandled rejections.

The reviewer also confirmed the evidence:

• The two new server tests fail on main (413, and a 5 s timeout) and pass on this branch.
• Removing resumeConnectionTraces() from api.ts fails the sign-in tests.

[devswha]

Merge pass review-work report. Overall: PASSED.

Lane	Verdict	Confidence
Manual QA (artifacts kept)	PASS	-
Gate review (another model; programming + remove-ai-slops applied directly)	APPROVE	HIGH

QA

• 46 tests pass across the changed files and the conventions tests, and the typecheck passes.
• git merge-tree is clean against main.
• It is also clean against the five maintainer PRs that share shared/protocol.ts, where this PR changes only a comment: fix(input): validate immediate submit paste framing #1840, feat(tasks): inspect running Claude subagent details #1842, feat(chat): fold contiguous todo snapshots without losing attempts #1846, feat(terminal): paste into the input line and insert a line without Enter #1853 and feat(changes): review a branch against a chosen base #1857.

Gate: real HTTP probes against an isolated server

• Trickled bodies: all 16 got a complete 408 trace_timeout after 10.020-10.022 s, and the next honest report got 204.
• A stalled body: got 408 after 10.003 s and released its slot.
• On-time reports: still get 204.
• Timers and slots: 24 timers were created and 24 cleared, and no slot was released twice.
• Client: it drops a 408 batch without retrying and reports the next incident normally.
• Regression scenarios: both fail against the pre-fix code, with 413 instead of 408, and stalled slots go on returning 429.

All required checks are green and no review thread is open.

CodeRabbit did not review this PR (rate-limited or skipped). The reviews above stand in for it.