Skip to content

fix(server): reject out-of-range ListTasks cursor timestamps - #1451

Merged
Iwaniukooo11 merged 2 commits into
a2aproject:mainfrom
lj1166:fix/invalid-cursor-timestamp
Nov 13, 2026
Merged

Iwaniukooo11 merged 2 commits into
a2aproject:mainfrom
lj1166:fix/invalid-cursor-timestamp

Conversation

@lj1166

@lj1166 lj1166 commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #1450.

ListTasks with DatabaseTaskStore returns HTTP 500 when a structurally valid page token contains an out-of-range timestamp. Protobuf 7.36.2 raises ValueError from Timestamp.FromNanoseconds(), while _after_cursor only converts OverflowError into InvalidParamsError.

Handle both exceptions so these tokens use the existing HTTP 400 / INVALID_ARGUMENT / Invalid page token response. The production change is one exception clause. Add four regression cases covering values immediately outside the timestamp range and very large positive/negative integers.

Verification

Tested source main at 2cc1dd5, Python 3.12.16, Windows, Protobuf 7.36.2, and SQLite/aiosqlite. Final checks used the exact dependencies exported from uv.lock with hash verification.

  • Before the fix, all four new SQLite cases failed with ValueError; after the fix they pass. PostgreSQL/MySQL variants skip because their DSNs are unset.
  • Exercised real Starlette REST routes, DefaultRequestHandlerV2, and an in-memory SQLite DatabaseTaskStore via httpx.ASGITransport, without mocking the conversion or store: three invalid timestamps changed from HTTP 500 to HTTP 400; ts=null and ts=0 remained HTTP 200. No provider requests.
  • scripts/lint.sh: passed Ruff check/format and ty. ty reports three existing unused-ignore warnings in untouched client files; no type errors.
  • The suite with the two Windows-incompatible integration modules below excluded: 2,556 passed, 215 skipped, 4 xfailed, 1 xpassed.
  • The same suite with --cov=src --cov-report=term-missing --cov-fail-under=88: 93.57% coverage, same test counts.
  • git diff --check: passed.

The passing suite command (with UTF-8 enabled for sample CLI subprocesses):

PYTHONUTF8=1 uv run pytest --timeout=45 -q \
  --ignore=tests/integration/cross_version/client_server/test_client_server.py \
  --ignore=tests/integration/push_notifications/test_default_push_notification_support.py

Full-suite limitations

I also ran the unfiltered suite; it was not green on this Windows host. Its initial result was 2,553 passed, 215 skipped, 4 xfailed, 1 xpassed, 3 failed, and 8 errors. The three sample CLI failures were GBK console-encoding errors and passed with PYTHONUTF8=1.

The two excluded modules fail on the unchanged production source too (verified by restoring the original exception clause and rerunning them): the cross-version harness uses POSIX os.killpg and :-joined PYTHONPATH and has short process startup/client timeouts; the push-notification harness cannot pickle a locally created FastAPI application under Windows spawn (FastAPI.setup.<locals>.openapi). I left these unrelated harnesses unchanged. Linux, PostgreSQL, and MySQL runs have not been performed locally.

Checklist

  • Followed CONTRIBUTING.md and read the repository's AI workflow documents.
  • Conventional Commit title and focused bug fix with regression coverage.
  • Ran the lint script, unfiltered tests, and the passing suite/coverage commands described above.
  • No documentation update needed: no public API or schema change.

AI disclosure: Codex investigated the code, reproduced the issue, prepared and reviewed this patch, and ran the checks under the account holder's instruction. Independent human review or reproduction is not claimed.

@lj1166
lj1166 requested a review from a team as a code owner November 12, 2026 03:34
@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/tasks/database_task_store.py 95.70% 96.77% 🟢 +1.08%
Total 93.57% 93.59% 🟢 +0.02%

Generated by coverage-comment.yml

@ishymko
ishymko requested review from a team and removed request for a team November 12, 2026 10:27

@1AbdullahButt 1AbdullahButt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct. Here's what I checked.

  • _ns_to_datetime is the only user-controlled path. It's called once, from _after_cursor (database_task_store.py:347). _cursor_for goes the other way, from the stored column, so it can't produce an out-of-range value. Catching (OverflowError, ValueError) around that single call covers every token that decode_list_tasks_cursor accepts. decode_list_tasks_cursor (utils/task.py:180-187) only checks that ts is a non-bool int or None, and has no range check.
  • Both exception types need catching across the supported protobuf range. pyproject.toml allows protobuf>=5.29.5. The description shows 7.x raising ValueError from FromNanoseconds. The existing OverflowError clause covers the ToDatetime() side. Keeping both is right, because which call fails depends on the version and backend.
  • The test bounds are exact. -62_135_596_800_000_000_001 is 1 ns before 0001-01-01T00:00:00Z, and 253_402_300_800_000_000_000 is exactly 10000-01-01T00:00:00Z, 1 ns past the largest valid Timestamp. Those are the limits in the Timestamp definition ("0001-01-01T00:00:00Z to 9999-12-31T23:59:59.999999999Z").
  • InMemoryTaskStore doesn't need the same fix. It compares the integer sort key and never builds a datetime from the cursor.
  • git merge-tree against main is clean.

A small optional extension: a boundary case just inside the range (253_402_300_799_999_999_999) that still returns a page would pin that the check isn't too strict.

I didn't run the tests.

Reviewed with help from Claude Code.

@Iwaniukooo11 Iwaniukooo11 self-assigned this Nov 13, 2026

@Iwaniukooo11 Iwaniukooo11 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Iwaniukooo11
Iwaniukooo11 merged commit f451ed7 into a2aproject:main Nov 13, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ListTasks returns HTTP 500 for an out-of-range cursor timestamp

3 participants