Repository navigation
fix(server): reject out-of-range ListTasks cursor timestamps - #1451
Merged
Iwaniukooo11 merged 2 commits intoNov 13, 2026
Merged
Conversation
🧪 Code Coverage (vs
|
| 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
1AbdullahButt
left a comment
Contributor
There was a problem hiding this comment.
Looks correct. Here's what I checked.
_ns_to_datetimeis the only user-controlled path. It's called once, from_after_cursor(database_task_store.py:347)._cursor_forgoes 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 thatdecode_list_tasks_cursoraccepts.decode_list_tasks_cursor(utils/task.py:180-187) only checks thattsis a non-boolintorNone, and has no range check.- Both exception types need catching across the supported protobuf range.
pyproject.tomlallowsprotobuf>=5.29.5. The description shows 7.x raisingValueErrorfromFromNanoseconds. The existingOverflowErrorclause covers theToDatetime()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_001is 1 ns before0001-01-01T00:00:00Z, and253_402_300_800_000_000_000is exactly10000-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"). InMemoryTaskStoredoesn't need the same fix. It compares the integer sort key and never builds adatetimefrom the cursor.git merge-treeagainstmainis 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1450.
ListTaskswithDatabaseTaskStorereturns HTTP 500 when a structurally valid page token contains an out-of-range timestamp. Protobuf 7.36.2 raisesValueErrorfromTimestamp.FromNanoseconds(), while_after_cursoronly convertsOverflowErrorintoInvalidParamsError.Handle both exceptions so these tokens use the existing HTTP 400 /
INVALID_ARGUMENT/Invalid page tokenresponse. 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
mainat2cc1dd5, Python 3.12.16, Windows, Protobuf 7.36.2, and SQLite/aiosqlite. Final checks used the exact dependencies exported fromuv.lockwith hash verification.ValueError; after the fix they pass. PostgreSQL/MySQL variants skip because their DSNs are unset.DefaultRequestHandlerV2, and an in-memory SQLiteDatabaseTaskStoreviahttpx.ASGITransport, without mocking the conversion or store: three invalid timestamps changed from HTTP 500 to HTTP 400;ts=nullandts=0remained 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.--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):
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.killpgand:-joinedPYTHONPATHand 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
CONTRIBUTING.mdand read the repository's AI workflow documents.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.