Skip to content

fix(server): supply missing task status timestamps - #1346

Open
Linux2010 wants to merge 3 commits into
a2aproject:mainfrom
Linux2010:fix/1331-failed-timestamp
Open

Linux2010 wants to merge 3 commits into
a2aproject:mainfrom
Linux2010:fix/1331-failed-timestamp

Conversation

@Linux2010

@Linux2010 Linux2010 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #1331. Framework failure paths could persist a FAILED task without a status.timestamp, leaving DatabaseTaskStore.last_updated null and affecting task ordering and timestamp filtering.

  • Stamp FAILED statuses at the framework failure sites.
  • In TaskManager, supply a timestamp for a missing timestamp on a status in a raw Task or status-update event, including non-terminal states. Keep an agent-provided timestamp unchanged and stamp the event before copying its status to the stored task.
  • Timestamp the initial SUBMITTED status created by TaskManager, including return-immediately and artifact-first paths.

This supplies timestamps for statuses passing through these paths; it does not migrate existing database records. Artifact-only updates to an old task with an unstamped status do not backfill that existing status.

Validation

  • ./scripts/lint.sh (passed; three pre-existing ty unused-ignore warnings).
  • uv run pytest (2,620 passed, 217 skipped, 4 xfailed, 1 xpassed before the additional raw-WORKING test).
  • uv run pytest --cov=src --cov-report=term-missing (2,621 passed, 217 skipped, 4 xfailed, 1 xpassed; 94% coverage).
  • Tests cover non-terminal and terminal status events, raw tasks, preserved timestamps, SUBMITTED, return-immediately, artifact-first, and SQLite status_timestamp_after filtering. PostgreSQL and MySQL cases were skipped because their DSNs were not configured.

Prepared with AI assistance.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/agent_execution/active_task.py 96.69% 96.70% 🟢 +0.01%
src/a2a/server/request_handlers/default_request_handler_v2.py 88.60% 88.63% 🟢 +0.03%
src/a2a/server/tasks/database_task_store.py 96.77% 98.39% 🟢 +1.61%
src/a2a/server/tasks/task_manager.py 98.95% 98.99% 🟢 +0.05%
Total 93.63% 93.67% 🟢 +0.04%

Generated by coverage-comment.yml

@mykytanetipa mykytanetipa self-assigned this Oct 19, 2026
@Linux2010
Linux2010 force-pushed the fix/1331-failed-timestamp branch from c34c53f to 6be10db Compare October 21, 2026 02:00
@Linux2010

Copy link
Copy Markdown
Contributor Author

Friendly ping — this is ready for review whenever anyone has bandwidth (both branches just rebased onto latest main). Happy to address any feedback. Thanks!

@mykytanetipa mykytanetipa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why only stamp terminal states in TaskManager?

  • If an agent sends an unstamped WORKING or INPUT_REQUIRED update (for example via new_text_status_update_event, which doesn't set a timestamp), task.status.CopyFrom(event.status) still wipes out the existing timestamp and writes last_updated = NULL to the DB.

  • New SUBMITTED tasks created via new_task_from_user_message or TaskManager._init_task_obj (which ensure_task_id saves directly through _save_task) also start without a timestamp, so tasks created with return_immediately=True or artifact-first flows end up with last_updated = NULL too.

I think we should stamp any status that lacks a timestamp (in save_task_event and _init_task_obj / _save_task), not just terminal ones

Comment thread src/a2a/server/tasks/task_manager.py Resolved
Linux2010 added a commit to Linux2010/a2a-python that referenced this pull request Nov 2, 2026
Most task-lifecycle defects in this SDK are not wrong values: they are a
correct value that reached one surface and not the others (persisted store,
streaming subscribers, push notifications, the cluster task_events table,
and the two separate v0.3 compat mappers). The existing rule layer routes
mistakes to mistake-reflection and code style to docs/ai, but nothing tells
a change agent which surfaces a status write must reach, so the same class
keeps recurring: a2aproject#1276 (status.timestamp mapped for JSON-RPC/REST compat but
not gRPC), a2aproject#1188 (handler resumes from a stale snapshot), a2aproject#1206/a2aproject#1411
(contextId inference disagrees between events and the stored task), and the
review findings on a2aproject#1346 (stamping the copy instead of the event, and missing
the CANCELED paths).

Adds .agents/skills/task-state-invariants/SKILL.md with the invariant set
(each claim anchored to current main), the sibling-surface checklist, the
matching test files per surface, and the evidence expected in a PR, plus a
one-line router pointer in AGENTS.md so agents actually load it.

Signed-off-by: andy <[email protected]>
@Linux2010
Linux2010 force-pushed the fix/1331-failed-timestamp branch from 6be10db to c923c37 Compare November 3, 2026 16:21
@Linux2010

Linux2010 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

All review threads here are addressed and resolved (rebased + fixes pushed as c923c37). Thanks for the detailed review — the CANCELED paths and the event-vs-copy stamping order were the two things I had missed.

Summary of what changed after your review:

  • rebased onto current main; _write_failed_status and the producer path now defer to fix(server): record executor failures through the event consumer #1366, and the _now_timestamp() helper is gone in favor of direct datetime.now(timezone.utc) calls;
  • ActiveTask.cancel() and DefaultRequestHandlerV2._write_cancel() now stamp CANCELED explicitly (CopyFrom was carrying the previous state's timestamp);
  • the TaskManager backstop stamps event.status before CopyFrom, so subscribers, push notifications and the task_events table get the timestamp too, not just the stored task;
  • TERMINAL_TASK_STATES is imported from a2a.utils.constants rather than re-declared locally;
  • new test test_save_task_event_stamps_the_published_event pins the event/task timestamp equality.

Local: ruff + ty clean, 2526 passed, coverage 94%. The 4 cross_version integration errors are environmental (identical on a clean main checkout in this sandbox).

@Linux2010
Linux2010 force-pushed the fix/1331-failed-timestamp branch from c923c37 to 3b4f247 Compare November 12, 2026 01:32
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 <[email protected]>
@Linux2010
Linux2010 force-pushed the fix/1331-failed-timestamp branch from 3b4f247 to a5b7d92 Compare November 12, 2026 10:52
@Linux2010
Linux2010 requested a review from a team as a code owner November 12, 2026 10:52
Linux2010 added a commit to Linux2010/a2a-python that referenced this pull request Nov 12, 2026
Most task-lifecycle defects in this SDK are not wrong values: they are a
correct value that reached one surface and not the others (persisted store,
streaming subscribers, push notifications, the cluster task_events table,
and the two separate v0.3 compat mappers). The existing rule layer routes
mistakes to mistake-reflection and code style to docs/ai, but nothing tells
a change agent which surfaces a status write must reach, so the same class
keeps recurring: a2aproject#1276 (status.timestamp mapped for JSON-RPC/REST compat but
not gRPC), a2aproject#1188 (handler resumes from a stale snapshot), a2aproject#1206/a2aproject#1411
(contextId inference disagrees between events and the stored task), and the
review findings on a2aproject#1346 (stamping the copy instead of the event, and missing
the CANCELED paths).

Adds .agents/skills/task-state-invariants/SKILL.md with the invariant set
(each claim anchored to current main), the sibling-surface checklist, the
matching test files per surface, and the evidence expected in a PR, plus a
one-line router pointer in AGENTS.md so agents actually load it.

Signed-off-by: andy <[email protected]>
Linux2010 added a commit to Linux2010/a2a-python that referenced this pull request Nov 12, 2026
Line anchors for the cancel-path CopyFrom, the inline push send, the
get_task wait, and the copy-on-yield caveat had drifted after the
13-commit rebase onto main (which also merged a2aproject#1345/a2aproject#1346 — the very
fixes several anchors point at). All eight anchors re-verified against
current code; claim 5 notes the caller-side guard merged via a2aproject#1345.

Signed-off-by: andy <[email protected]>
Linux2010 added a commit to Linux2010/a2a-python that referenced this pull request Nov 13, 2026
Most task-lifecycle defects in this SDK are not wrong values: they are a
correct value that reached one surface and not the others (persisted store,
streaming subscribers, push notifications, the cluster task_events table,
and the two separate v0.3 compat mappers). The existing rule layer routes
mistakes to mistake-reflection and code style to docs/ai, but nothing tells
a change agent which surfaces a status write must reach, so the same class
keeps recurring: a2aproject#1276 (status.timestamp mapped for JSON-RPC/REST compat but
not gRPC), a2aproject#1188 (handler resumes from a stale snapshot), a2aproject#1206/a2aproject#1411
(contextId inference disagrees between events and the stored task), and the
review findings on a2aproject#1346 (stamping the copy instead of the event, and missing
the CANCELED paths).

Adds .agents/skills/task-state-invariants/SKILL.md with the invariant set
(each claim anchored to current main), the sibling-surface checklist, the
matching test files per surface, and the evidence expected in a PR, plus a
one-line router pointer in AGENTS.md so agents actually load it.

Signed-off-by: andy <[email protected]>
Linux2010 added a commit to Linux2010/a2a-python that referenced this pull request Nov 13, 2026
Line anchors for the cancel-path CopyFrom, the inline push send, the
get_task wait, and the copy-on-yield caveat had drifted after the
13-commit rebase onto main (which also merged a2aproject#1345/a2aproject#1346 — the very
fixes several anchors point at). All eight anchors re-verified against
current code; claim 5 notes the caller-side guard merged via a2aproject#1345.

Signed-off-by: andy <[email protected]>
@Linux2010 Linux2010 changed the title fix(server): always stamp status.timestamp on terminal task states fix(server): supply missing task status timestamps Nov 14, 2026

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

2 participants