Skip to content

Commit e12d494

Browse files
anxkhnmykytanetipa
andauthored
fix(server): include legacy task states in database list filters (#1337)
# description `DatabaseTaskStore` can read v0.3 task rows, but its status filter compares their JSON state only with the 1.0 protobuf enum name. A legacy working task is readable with `GetTask` yet missing from `ListTasks(status=WORKING)`, with an incorrect total. Match both spellings using the existing compatibility converter. The filter stays in SQL before counting and pagination, with owner scoping unchanged. No schema change is needed. The regression covers all eight concrete states, mixed modern and legacy rows, missing legacy protocol versions, owner isolation and page tokens. All eight SQLite cases failed before the fix. ## validation Rebased onto main at `799fcae`. Fresh checks on that base: - `UV_PYTHON=3.10 ./scripts/lint.sh` passed Ruff, formatting and ty. - `UV_PYTHON=3.10 uv run pytest --timeout=45`: 2,277 passed, 199 skipped, 3 xfailed, 1 xpassed. - `UV_PYTHON=3.10 uv run pytest --timeout=45 --cov=src --cov-report=term-missing --cov-fail-under=88`: same test results, 93.29% coverage. PostgreSQL/MySQL service tests skipped because their DSNs were unset. The new regressions ran on SQLite. Existing xfail/xpass cases reference #869. - [x] Follow the [`CONTRIBUTING` Guide](https://github.com/a2aproject/a2a-python/blob/main/CONTRIBUTING.md). - [x] Make your Pull Request title in the <https://www.conventionalcommits.org/> specification. - [x] Ensure the tests and linter pass. - [x] Appropriate docs were updated if necessary. No documentation change is needed for this filter correction. Co-authored-by: mykytanetipa <mykytanetipa@google.com>
1 parent 05392b3 commit e12d494

2 files changed

Lines changed: 87 additions & 2 deletions

File tree

‎src/a2a/server/tasks/database_task_store.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
from google.protobuf.json_format import MessageToDict, ParseDict
2626
from google.protobuf.timestamp_pb2 import Timestamp
2727

28+
from a2a.compat.v0_3.conversions import to_compat_task_status
2829
from a2a.compat.v0_3.model_conversions import (
2930
compat_task_model_to_core,
3031
)
@@ -259,8 +260,16 @@ async def list(
259260
)
260261
if params.status:
261262
base_stmt = base_stmt.where(
262-
self.task_model.status['state'].as_string()
263-
== a2a_pb2.TaskState.Name(params.status)
263+
self.task_model.status['state']
264+
.as_string()
265+
.in_(
266+
[
267+
a2a_pb2.TaskState.Name(params.status),
268+
to_compat_task_status(
269+
a2a_pb2.TaskStatus(state=params.status)
270+
).state.value,
271+
]
272+
)
264273
)
265274
if params.HasField('status_timestamp_after'):
266275
last_updated_after = params.status_timestamp_after.ToDatetime()

‎tests/server/tasks/test_database_task_store.py‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,82 @@ def capture_bound_datetimes(
407407
assert all(value.year >= 1000 for value in bound_datetimes)
408408

409409

410+
@pytest.mark.asyncio
411+
@pytest.mark.parametrize(
412+
'state, legacy_state',
413+
[
414+
(TaskState.TASK_STATE_SUBMITTED, 'submitted'),
415+
(TaskState.TASK_STATE_WORKING, 'working'),
416+
(TaskState.TASK_STATE_COMPLETED, 'completed'),
417+
(TaskState.TASK_STATE_FAILED, 'failed'),
418+
(TaskState.TASK_STATE_CANCELED, 'canceled'),
419+
(TaskState.TASK_STATE_INPUT_REQUIRED, 'input-required'),
420+
(TaskState.TASK_STATE_REJECTED, 'rejected'),
421+
(TaskState.TASK_STATE_AUTH_REQUIRED, 'auth-required'),
422+
],
423+
)
424+
async def test_list_tasks_filters_mixed_protocol_states(
425+
db_store_parameterized: DatabaseTaskStore,
426+
state: TaskState.ValueType,
427+
legacy_state: str,
428+
) -> None:
429+
store = db_store_parameterized
430+
other_context = ServerCallContext(user=SampleUser('other_user'))
431+
for owner_context in (TEST_CONTEXT, other_context):
432+
owner = store.owner_resolver(owner_context)
433+
for prefix, core_state, stored_state in (
434+
('match', state, legacy_state),
435+
('nonmatch', TaskState.TASK_STATE_UNSPECIFIED, 'unknown'),
436+
):
437+
await store.save(
438+
Task(
439+
id=f'{owner}-{prefix}-modern',
440+
context_id='mixed-context',
441+
status=TaskStatus(state=core_state),
442+
),
443+
owner_context,
444+
)
445+
async with store.async_session_maker.begin() as session:
446+
for version in (None, '0.3'):
447+
await session.execute(
448+
insert(store.task_model).values(
449+
id=f'{owner}-{prefix}-legacy-{version}',
450+
context_id='mixed-context',
451+
owner=owner,
452+
status={'state': stored_state},
453+
protocol_version=version,
454+
)
455+
)
456+
457+
owner = store.owner_resolver(TEST_CONTEXT)
458+
expected_ids = [
459+
f'{owner}-match-modern',
460+
f'{owner}-match-legacy-None',
461+
f'{owner}-match-legacy-0.3',
462+
]
463+
for params in (
464+
ListTasksRequest(),
465+
ListTasksRequest(status=TaskState.TASK_STATE_UNSPECIFIED),
466+
):
467+
unfiltered = await store.list(params, TEST_CONTEXT)
468+
assert unfiltered.total_size == 6
469+
assert {task.id for task in unfiltered.tasks} == {
470+
task_id.replace('-match-', f'-{prefix}-')
471+
for task_id in expected_ids
472+
for prefix in ('match', 'nonmatch')
473+
}
474+
475+
params = ListTasksRequest(status=state, page_size=1)
476+
for index, expected_id in enumerate(expected_ids):
477+
page = await store.list(params, TEST_CONTEXT)
478+
assert page.total_size == 3
479+
assert [task.id for task in page.tasks] == [expected_id]
480+
assert page.tasks[0].status.state == state
481+
assert page.page_size == 1
482+
assert bool(page.next_page_token) == (index < len(expected_ids) - 1)
483+
params.page_token = page.next_page_token
484+
485+
410486
@pytest.mark.asyncio
411487
@pytest.mark.parametrize(
412488
'params, expected_error_message',

0 commit comments

Comments
 (0)