Skip to content

fix(sessions): preserve Dapr session created_at across writes#3236

Draft
adityasingh2400 wants to merge 2 commits into
openai:mainfrom
adityasingh2400:fix/dapr-preserve-created-at
Draft

fix(sessions): preserve Dapr session created_at across writes#3236
adityasingh2400 wants to merge 2 commits into
openai:mainfrom
adityasingh2400:fix/dapr-preserve-created-at

Conversation

@adityasingh2400
Copy link
Copy Markdown
Contributor

Summary

DaprSession.add_items() rewrote the session metadata on every call, clobbering created_at with the current timestamp on each write. The field now reflects the true session start time, while updated_at advances as expected.

  • Read back the existing metadata before writing and reuse the prior created_at if it parses as a dict with that field.
  • Fall back to the current timestamp only when metadata is missing or corrupt (handled via guarded JSON / decode error catch).
  • Drop a duplicated etag = getattr(response, "etag", None) or None line in pop_item() (dead code).

Test plan

  • New test_metadata_preserves_created_at asserts created_at is stable across two add_items calls one second apart while updated_at advances.
  • Full tests/extensions/memory/ suite passes (180 passed, 5 skipped).
  • ruff check and ruff format --check clean on touched files.

@github-actions github-actions Bot added bug Something isn't working feature:extensions labels May 8, 2026
@seratch
Copy link
Copy Markdown
Member

seratch commented May 8, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cf43e322e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 347 to 348
await self._dapr_client.save_state(
store_name=self._state_store_name,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard metadata update with an ETag

This fix reads existing metadata and then overwrites the metadata key without any ETag/concurrency option. With the default eventual reads or two DaprSession instances writing the same session, a stale/missing metadata read leaves created_at = now, and this unconditional save can clobber an older created_at, so the field can still advance across writes despite the commit intent.

Useful? React with 👍 / 👎.

@seratch seratch marked this pull request as draft May 8, 2026 17:03
DaprSession.add_items() rewrote the session metadata on every call,
clobbering created_at with the current timestamp. The field is now read
back from the metadata key and only initialized on first write, so
created_at reflects the true session start while updated_at advances.
Also removes a duplicated etag assignment in pop_item().
The metadata read-modify-write was unconditional, so two writers (or one
writer with a stale eventual read) could clobber an existing created_at.
Apply the same first-write concurrency + retry-on-conflict pattern used
for the messages key so the field is preserved across concurrent writes.
@adityasingh2400 adityasingh2400 force-pushed the fix/dapr-preserve-created-at branch from 3ce6b86 to 14b11ab Compare May 11, 2026 23:47
@seratch seratch removed the bug Something isn't working label May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants