Skip to content

fix(core): tolerate disappearing guarded files - #1383

Merged
phernandez merged 6 commits into
mainfrom
logfire-core-fixes-20260830
Aug 30, 2026
Merged

fix(core): tolerate disappearing guarded files#1383
phernandez merged 6 commits into
mainfrom
logfire-core-fixes-20260830

Conversation

@phernandez

@phernandez phernandez commented Aug 30, 2026

Copy link
Copy Markdown
Member

Why

A hosted note object can be deleted after the materializer confirms it exists but before it reads the checksum. That ordinary object-store race currently escapes as a terminal checksum failure and poisons every durable materialization retry.

Production Logfire issue #2533 captured the exact sequence: the S3 existence probe succeeded, GetObject returned NoSuchKey, and the materialization job failed instead of treating the destination as absent.

What Changed

  • Treat FileNotFoundError from the checksum read after a successful existence probe as an absent runtime file.
  • Preserve the general FileService FileError contract for direct callers, and translate only a missing-file cause in the local materialization and directory-delete storage adapters.
  • Preserve every other checksum failure as actionable.
  • Add focused regressions using both real runtime adapters, including both full guarded-delete paths, to model post-probe deletion and preserve the direct service contract.

Implementation Details

The portable guard remains lock-free. It does not retry or introduce a second probe: absence before the probe and absence during the checksum read have the same domain result, so both return None to the existing conflict planner.

Cloud's S3 adapter change is delivered in basic-memory-cloud PR #1893 so NoSuchKey crosses this boundary as FileNotFoundError.

Testing

  • uv run pytest tests/runtime/test_note_file_guard_races.py tests/services/test_file_service.py tests/repository/test_chunk_inspection.py -q: 59 passed, 1 skipped.
  • Focused Ruff and ty checks for the changed source and test: passed.
  • git diff --check: passed.
  • Load-bearing check: removing the runtime-adapter translation makes the real-adapter race regression fail.

The broad just fast-check reached type checking but the fresh worktree lacks the optional pymilvus dependency; the four failures are unresolved optional imports outside these paths. CI installs the repository's required matrix dependencies.

Risks / Follow-ups

  • This deliberately handles only FileNotFoundError; permissions, transport failures, malformed content, and other checksum errors still fail.
  • Codex's reviews correctly identified the fake-only coverage gap, the direct FileService contract regression, and the final-checksum seams in both local delete adapters; commits through aeb09978 address those findings and the threads are resolved.
  • No deployment or production mutation is included.

Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-30T06:37:39.476975Z aeb0997 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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

Copy link
Copy Markdown

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: fae73a4f0d

ℹ️ 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 thread src/basic_memory/runtime/note_file_guards.py
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

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

Copy link
Copy Markdown

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: 3a9ecaf4c9

ℹ️ 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 thread src/basic_memory/services/file_service.py Outdated
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

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

Copy link
Copy Markdown

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: 5dbdf95514

ℹ️ 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 thread src/basic_memory/runtime/note_file_guards.py
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

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

Copy link
Copy Markdown

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: 88e13811f6

ℹ️ 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 thread src/basic_memory/index/local_notes.py
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

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

Copy link
Copy Markdown

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: 14dc261fdc

ℹ️ 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 thread src/basic_memory/index/note_content_materialization.py
Signed-off-by: phernandez <paul@basicmachines.co>
@phernandez

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: aeb09978d4

ℹ️ 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".

@phernandez
phernandez merged commit a4b2570 into main Aug 30, 2026
28 checks passed
@phernandez
phernandez deleted the logfire-core-fixes-20260830 branch August 30, 2026 07:07
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.

1 participant