Backend: clock skew tolerance + JSON 408 diagnostics (#5929)

[beastoin]

Summary

Backend sub-PR for issue #5929 collab. Adds clock skew tolerance to TimeoutMiddleware and returns JSON 408 with diagnostic info.

Changes

backend/utils/other/timeout.py:

• HTTP_CLOCK_SKEW_ALLOWANCE env var (default 5min) added to stale threshold
• Effective threshold = max_age (5min) + skew_allowance (5min) = 10min
• 408 returns JSON: {error, message, server_time, client_time, skew_seconds, hint}

backend/tests/unit/test_timeout_middleware.py — 12 tests

Ownership

• kenji owns: backend/utils/other/timeout.py, backend/tests/unit/test_timeout_middleware.py
• kelvin owns: app/ files (separate sub-PR)

Status

• 12 unit tests pass
• Codex reviewer: PR_APPROVED_LGTM
• Codex tester: TESTS_APPROVED

Fixes #5929 (backend portion)

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR extends TimeoutMiddleware with clock-skew tolerance and richer 408 diagnostics, addressing the root cause of spurious request rejections from mobile clients whose clocks drift behind the server. The effective stale threshold becomes max_age + skew_allowance (default 10 min total), and a rejected request now returns structured JSON with server_time, client_time, skew_seconds, and a user-facing hint rather than a bare 408 text body.

Key observations:

• The clock skew allowance is correctly read once at middleware construction time (not per-request), so it's efficient and env-var overrides work cleanly via monkeypatch.
• The skew_seconds field in the 408 payload is actually the full apparent request age (current_time − client_reported_time), not the isolated clock skew. Clients parsing this field to display a "your clock is X seconds off" message may mislead users; consider renaming to apparent_age_seconds or adjusting the value to request_age − maximum_age_seconds.
• import os in the test file is unused and can be removed.
• Test coverage (12 tests) is thorough, hitting boundary conditions, env-var overrides, malformed/missing/future headers, multipart uploads, and the 504 timeout path.

Confidence Score: 4/5

• This PR is safe to merge; the two remaining notes are non-blocking style suggestions.
• The core logic is correct, the feature works as described, and test coverage is comprehensive. The only open items are a misleading JSON field name (skew_seconds vs. apparent age) and a single unused import — neither is a runtime bug nor a data-loss/security risk.
• No files require special attention; backend/utils/other/timeout.py has one minor naming concern in the JSON response payload.

Important Files Changed

Filename	Overview
backend/utils/other/timeout.py	Adds clock skew tolerance (configurable via HTTP_CLOCK_SKEW_ALLOWANCE, default 5 min) to the stale-request check, and replaces the plain-text 408 with a structured JSON diagnostic payload. Logic is sound; minor issue: the skew_seconds JSON field contains the full request apparent age, not the isolated clock skew.
backend/tests/unit/test_timeout_middleware.py	12 well-structured unit tests covering fresh requests, boundary conditions, clock skew tolerance, multipart uploads, malformed/missing/future headers, env-var overrides, and the 504 timeout path. One unused import os on line 13.

Sequence Diagram

sequenceDiagram
    participant Client
    participant TimeoutMiddleware
    participant App

    Client->>TimeoutMiddleware: Request + X-Request-Start-Time header
    TimeoutMiddleware->>TimeoutMiddleware: request_age = server_time - client_time
    alt request_age > max_age + skew_allowance (default 10 min)
        TimeoutMiddleware-->>Client: 408 JSON {error, server_time, client_time, skew_seconds, hint}
    else header absent or malformed
        TimeoutMiddleware->>App: pass through
    else request_age within threshold
        TimeoutMiddleware->>App: pass through
        App-->>TimeoutMiddleware: response
        TimeoutMiddleware-->>Client: response
    end
    note over TimeoutMiddleware: asyncio.wait_for wraps call_next
    alt execution exceeds per-method timeout
        TimeoutMiddleware-->>Client: 504 Gateway Timeout
    end

Loading

_{Reviews (1): Last reviewed commit: "Add unit tests for clock skew tolerance ..." | Re-trigger Greptile}

[greptile-apps]

Unused import

os is imported but never referenced in the test file — all env manipulation goes through monkeypatch.setenv(). Safe to remove.

Suggested change

	import os
	import asyncio

[greptile-apps]

skew_seconds field contains request age, not clock skew

round(request_age, 1) is the apparent age of the request from the server's perspective (i.e. current_time - client_reported_time). This includes both the genuine elapsed time since the request was sent and any clock skew component; it isn't the clock skew in isolation.

For a request that took 5 s to arrive with a 650 s skew, request_age would be ~655 s, but the actual skew is 650 s. Labelling this skew_seconds is semantically misleading for API consumers (e.g. mobile apps parsing the JSON to display a friendly error). Consider renaming to apparent_age_seconds (or similar) to be accurate, and optionally add a separate field for the estimated skew if needed:

Suggested change

	"skew_seconds": round(request_age, 1),
	"skew_seconds": round(request_age - self.maximum_age_seconds, 1),

or rename:

                            "apparent_age_seconds": round(request_age, 1),

The test assertion on line 84 (body["skew_seconds"] >= 900) also asserts the request age rather than the skew, confirming the current semantics.