Skip to content

fix(server): always stamp status.timestamp on terminal task states - #1346

Open
Linux2010 wants to merge 1 commit into
a2aproject:mainfrom
Linux2010:fix/1331-failed-timestamp
Open

Linux2010 wants to merge 1 commit into
a2aproject:mainfrom
Linux2010:fix/1331-failed-timestamp

Conversation

@Linux2010

Copy link
Copy Markdown
Contributor

Description

Framework failure paths could persist a FAILED task without status.timestamp, and DatabaseTaskStore derives last_updated from status.timestamp (database_task_store.py:152-156) — so the task was persisted with last_updated = NULL, breaking ListTasks ordering and status_timestamp_after filtering. Root-cause analysis and a runtime reproduction are in #1331.

Per the two-layer approach endorsed on the issue:

  • Failure sites stamp explicitly (ActiveTask): _write_failed_status, _mark_task_as_failed, and the producer failure path now stamp the current time on the FAILED status.
  • Defensive backstop in TaskManager.save_task_event: a terminal state arriving without a timestamp (raw Task path or status-event path) is auto-stamped with the current time, so future call sites cannot reintroduce the NULL-last_updated hole.

Semantics: non-terminal states are never touched, and explicitly provided timestamps are never overwritten.

  • Follow the CONTRIBUTING Guide
  • PR title follows Conventional Commits (fix: → SemVer patch)
  • Tests and linter pass
  • Appropriate docs were updated (guard docstring states the contract)

Fixes #1331 🦕

Validation

  • Claim: an executor crashing mid-flight now persists TASK_STATE_FAILED with status.timestamp set; non-terminal statuses and explicitly provided timestamps are untouched.
  • Exact head: c34c53f
  • BEFORE: persisted task = state=TASK_STATE_FAILED, timestamp set=False (repro in [Bug]: Framework-written FAILED status drops status.timestamp, persisting last_updated as NULL #1331). AFTER: timestamp set=True.
  • Tests: 4 new tests in tests/server/tasks/test_task_manager.py (terminal-unstamped auto-stamp, provided-timestamp preserved, non-terminal untouched, raw-Task path guarded) + 1 end-to-end in test_default_request_handler_v2.py.
  • Full suite: 2302 passed, coverage 93% (gate 88%).
  • Environment: Python 3.10.20, macOS.
  • Limits / not tested: DatabaseTaskStore integration (covered indirectly via the same status pipeline; the e2e asserts the in-memory store the same way CI's coverage report measures it).

Prepared with AI assistance (WorkBuddy); reviewed and verified by Linux2010

Framework failure paths could persist a FAILED task without
status.timestamp: the consumer failure path built a
TaskStatusUpdateEvent with an unstamped TaskStatus (which
TaskManager.save_task_event then CopyFrom'd over the task, clearing
any prior timestamp), and the producer failure path mutated
task.status.state directly before saving. DatabaseTaskStore derives
last_updated from status.timestamp, so the task was persisted with
last_updated = NULL, breaking ListTasks ordering and time-based
filters (issue a2aproject#1331).

Per the approach endorsed on the issue, this lands both layers:

- failure sites in ActiveTask (_write_failed_status,
  _mark_task_as_failed, and the producer failure path) stamp the
  current time explicitly;
- TaskManager.save_task_event gains a defensive backstop that
  auto-populates the timestamp whenever a terminal state arrives
  unstamped, covering raw Task and status-event paths alike.

Non-terminal states are left untouched, and explicitly provided
timestamps are never overwritten.

Signed-off-by: andy <linux2011@qq.com>
@github-actions

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/agent_execution/active_task.py 95.40% 95.45% 🟢 +0.05%
src/a2a/server/cluster/database_event_stream.py 92.86% 96.94% 🟢 +4.08%
src/a2a/server/tasks/task_manager.py 98.86% 98.92% 🟢 +0.06%
Total 93.13% 93.18% 🟢 +0.05%

Generated by coverage-comment.yml

This branch has not been deployed

No deployments
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]: Framework-written FAILED status drops status.timestamp, persisting last_updated as NULL

1 participant