What time is it !

[fcenedes]

Attempt to tackle #71 - various approach possible. here is one

Summary

Implements timestamp validation for messages in working memory with backward compatibility. Clients are now expected to provide created_at timestamps for messages, with deprecation warnings for those who don't.

Closes #71

Problem

Currently, clients send messages to working memory without associated dates. The server auto-generates timestamps at deserialization time, which causes:

1. Incorrect message ordering - All messages in a batch get nearly identical timestamps
2. Lost temporal context - The actual time the message was created is lost
3. Inaccurate recency scoring - Long-term memory recency calculations use wrong timestamps

Solution

A phased approach with full backward compatibility:

Phase 1 (This PR) - Non-Breaking

• Log deprecation warnings when created_at is not provided (rate-limited)
• Add X-Deprecation-Warning response header to alert clients
• Validate that created_at is not in the future (with 5-minute clock skew tolerance)
• Add REQUIRE_MESSAGE_TIMESTAMPS config flag (default: false)

Phase 2 (Next Major Version)

• Change REQUIRE_MESSAGE_TIMESTAMPS default to true
• Messages without timestamps will be rejected with 400 error

Changes

Configuration (agent_memory_server/config.py)

• Added require_message_timestamps: bool = False - opt-in enforcement
• Added max_future_timestamp_seconds: int = 300 - clock skew tolerance (5 minutes)

Model Validation (agent_memory_server/models.py)

• Added model_validator(mode="before") to MemoryMessage class
• Rate-limited warnings using bounded in-memory set (max 10k entries)
• Future timestamp validation with configurable tolerance
• Added _created_at_was_provided private attribute for header logic

API Changes (agent_memory_server/api.py)

• Added X-Deprecation-Warning header to PUT /v1/working-memory/{session_id} when messages lack timestamps
• Extracted put_working_memory_core() for reuse by MCP

MCP Changes (agent_memory_server/mcp.py)

• Updated to use put_working_memory_core instead of put_working_memory

Client Changes (agent-memory-client/agent_memory_client/models.py)

• Mirrored validation logic for client-side consistency

Tests

• tests/test_models.py: Added TestMemoryMessageTimestampValidation (10 tests)
• tests/test_api.py: Added TestDeprecationHeader (4 tests)

Documentation

• docs/working-memory.md: Added timestamp guidance and deprecation notice
• docs/configuration.md: Added REQUIRE_MESSAGE_TIMESTAMPS and MAX_FUTURE_TIMESTAMP_SECONDS settings
• docs/api.md: Updated PUT endpoint example with created_at and deprecation header info

API Behavior

Default Mode (REQUIRE_MESSAGE_TIMESTAMPS=false)

# Request without created_at
curl -X PUT /v1/working-memory/session123 \
  -d '{"messages": [{"role": "user", "content": "Hello"}]}'

# Response includes deprecation header
# X-Deprecation-Warning: messages[].created_at will become required in the next major version...

Strict Mode (REQUIRE_MESSAGE_TIMESTAMPS=true)

# Request without created_at
curl -X PUT /v1/working-memory/session123 \
  -d '{"messages": [{"role": "user", "content": "Hello"}]}'

# Response: 422 Validation Error
# "created_at is required for messages..."

Future Timestamp Rejection

# Request with future timestamp (>5 min ahead)
curl -X PUT /v1/working-memory/session123 \
  -d '{"messages": [{"role": "user", "content": "Hello", "created_at": "2099-01-01T00:00:00Z"}]}'

# Response: 422 Validation Error
# "created_at cannot be more than 300 seconds in the future..."

Migration Guide for Clients

Before (deprecated)

client.set_working_memory(
    session_id="session123",
    messages=[
        {"role": "user", "content": "Hello"},  # No created_at
    ]
)

After (recommended)

from datetime import datetime, UTC

client.set_working_memory(
    session_id="session123",
    messages=[
        {
            "role": "user",
            "content": "Hello",
            "created_at": datetime.now(UTC).isoformat(),  # Provide timestamp
        },
    ]
)

Testing

# Run all tests
uv run pytest --run-api-tests

# Run specific tests for this feature
uv run pytest tests/test_models.py::TestMemoryMessageTimestampValidation -v
uv run pytest tests/test_api.py::TestDeprecationHeader -v

Test Results

• ✅ 539 passed, 30 skipped
• ✅ All pre-commit hooks pass

Checklist

• Backward compatible - existing clients continue to work
• Deprecation warnings logged (rate-limited)
• Deprecation header added to API responses
• Future timestamp validation with clock skew tolerance
• Config flag for opt-in strict enforcement
• Client model updated with same validation
• Unit tests added
• Integration tests pass
• Pre-commit hooks pass

[fcenedes]

the clock skew is configurable : MAX_FUTURE_TIMESTAMP_SECONDS

[Copilot]

Pull request overview

This PR implements timestamp validation for messages in working memory to address incorrect message ordering and lost temporal context. It introduces a phased, backward-compatible approach where clients are encouraged to provide created_at timestamps, with deprecation warnings for those who don't, and validation to prevent future timestamps beyond clock skew tolerance.

Key Changes

• Added optional timestamp validation with REQUIRE_MESSAGE_TIMESTAMPS config flag (default: false)
• Implemented deprecation warning system with rate-limited logging when timestamps are missing
• Added X-Deprecation-Warning response header to alert API clients
• Future timestamp validation with 5-minute clock skew tolerance

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
agent_memory_server/models.py	Added model_validator to MemoryMessage for timestamp validation, rate-limited warnings via class-level set, and _created_at_was_provided tracking
agent_memory_server/config.py	Added require_message_timestamps and max_future_timestamp_seconds settings
agent_memory_server/api.py	Refactored put_working_memory into core function and endpoint wrapper; added deprecation header logic
agent_memory_server/mcp.py	Updated to use put_working_memory_core instead of put_working_memory
agent-memory-client/agent_memory_client/models.py	Mirrored server validation logic without _created_at_was_provided tracking
tests/test_models.py	Added 10 tests for timestamp validation covering warnings, rate limiting, and future timestamp rejection
tests/test_api.py	Added 4 tests for deprecation header behavior in various scenarios
docs/working-memory.md	Added timestamp guidance, example usage with timestamps, and deprecation notice
docs/configuration.md	Documented timestamp validation settings and behavior
docs/api.md	Updated PUT endpoint example to include created_at timestamps

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The __init__ method checks for created_at in the data dictionary and stores the result in _created_at_was_provided. However, this logic duplicates the same check performed in the validate_created_at model_validator. Additionally, the __init__ is called after the validator runs (validators run during super().__init__()), so this creates a potential inconsistency.

Consider removing the custom __init__ and instead setting _created_at_was_provided within the validator itself by modifying the data dict to include a marker, or use a different approach like checking if the value equals the default at the API layer.

[Copilot]

The error message states "created_at cannot be in the future" but should be more specific: "created_at cannot be more than {tolerance} seconds in the future" to accurately reflect the validation logic. The current message is misleading since timestamps slightly in the future (within tolerance) are actually allowed.

Suggested change

	f"created_at cannot be in the future. "
	f"created_at cannot be more than {settings.max_future_timestamp_seconds} seconds in the future. "

[Copilot]

The error message states "created_at cannot be in the future" but should be more specific: "created_at cannot be more than {tolerance} seconds in the future" to accurately reflect the validation logic. The current message is misleading since timestamps slightly in the future (within tolerance) are actually allowed.

Suggested change

	f"created_at cannot be in the future. "
	f"Received: {created_at_value.isoformat()}, "
	f"Max allowed (with {cls._max_future_seconds}s tolerance): "
	f"{max_allowed.isoformat()}"
	f"created_at cannot be more than {cls._max_future_seconds} seconds in the future. "
	f"Received: {created_at_value.isoformat()}, "
	f"Max allowed: {max_allowed.isoformat()}"

[Copilot]

The documentation example uses datetime.now(UTC) for all three messages, which would give them identical timestamps. This defeats the purpose of providing timestamps for accurate message ordering. Consider using slightly different timestamps or adding a comment explaining that in real usage, timestamps should reflect actual message creation times.

[Copilot]

The class-level set _warned_message_ids is not thread-safe. In a multi-threaded or async environment (which FastAPI uses), concurrent access to check membership, check length, clear, and add operations can lead to race conditions. This could result in:

1. Duplicate warnings being logged
2. The set growing beyond _max_warned_ids
3. Messages being lost during clear operations

Consider using a thread-safe alternative like threading.Lock to protect these operations, or use a more robust rate-limiting mechanism like TTL-based caching (e.g., with functools.lru_cache or a dedicated rate-limiting library).

[Copilot]

The same thread-safety issue exists in the client model. The class-level set _warned_message_ids is not thread-safe, which could cause race conditions when multiple threads create MemoryMessage instances concurrently. Consider adding thread-safe protection or using a more robust rate-limiting mechanism.

[Copilot]

The documentation states that messages without created_at will be rejected with a "400 error" when REQUIRE_MESSAGE_TIMESTAMPS=true. However, Pydantic validation errors in FastAPI typically return 422 Unprocessable Entity status codes, not 400. Update this to "422 error" for accuracy, or verify that custom error handling returns 400.

Suggested change

	| `REQUIRE_MESSAGE_TIMESTAMPS=true` | Rejects messages without `created_at` with 400 error |
	| `REQUIRE_MESSAGE_TIMESTAMPS=true` | Rejects messages without `created_at` with 422 error |