Skip to content

fixes race condition in semanticcache#1304

Merged
akshaydeo merged 1 commit intov1.4.0from
01-11-fixes_race_condition_in_semanticcache
Jan 11, 2026
Merged

fixes race condition in semanticcache#1304
akshaydeo merged 1 commit intov1.4.0from
01-11-fixes_race_condition_in_semanticcache

Conversation

@akshaydeo
Copy link
Copy Markdown
Contributor

@akshaydeo akshaydeo commented Jan 11, 2026

Summary

Fix race condition in semantic cache plugin by retrieving context metadata before spawning a goroutine.

Changes

  • Moved the retrieval of paramsHash from context to before the goroutine is spawned
  • Added a comment explaining the importance of getting metadata from context before the goroutine to avoid race conditions when the same context is reused across multiple requests

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (Next.js)
  • Docs

How to test

Run the semantic cache plugin with concurrent requests using the same context:

go test ./plugins/semanticcache/... -race

Breaking changes

  • Yes
  • No

Related issues

Fixes potential race conditions when the semantic cache plugin processes multiple requests with the same context.

Security considerations

No security implications.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 11, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Fixed potential data races by capturing request parameters, IDs, and tags before launching background tasks, improving stability when contexts are reused.
    • Ensured tracing/generation tags and end-of-operation actions are only applied when the corresponding values are present, reducing incorrect or duplicated metadata.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Moved retrieval of context-derived metadata (paramsHash, generationID, traceID, tags) out of goroutines and into the calling scope so pre-captured values are used inside goroutines; added presence guards and conditional calls to end/propagate generation/trace only when corresponding values were present.

Changes

Cohort / File(s) Summary
Semantic cache goroutine fix
plugins/semanticcache/main.go
Pulled paramsHash retrieval out of the caching goroutine and used the pre-fetched value inside the goroutine to avoid data races when context is reused. Minor comment clarifications.
Generation/trace goroutine capture
plugins/maxim/main.go
Pre-captured generationID, traceID, and tags before spawning goroutine; replaced in-goroutine ctx.Value calls with captured values and presence flags; added guards so EndGeneration/EndTrace and tag propagation run only when IDs/tags were present.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • fixes race condition in semanticcache #1304 — Similar change in plugins/semanticcache/main.go: moved params metadata retrieval out of caching goroutine to avoid race.
  • maximhq/bifrost#... — (no additional strong matches found) — omitted.

🐰 I hopped ahead before the thread could fray,
I plucked the values safe and tucked them away,
Inside the goroutine they now stay sound,
No races chase, no tangles found,
A tidy burrow for code to play. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fixes race condition in semanticcache' accurately summarizes the primary change - fixing a race condition in the semantic cache plugin.
Description check ✅ Passed The PR description follows the template with all essential sections completed: Summary, Changes, Type of change (Bug fix selected), Affected areas (Plugins selected), How to test, Breaking changes, Related issues, and Security considerations. Checklist items are mostly marked.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 01-11-fixes_race_condition_in_semanticcache

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c766a8a and 90b4e04.

📒 Files selected for processing (2)
  • plugins/maxim/main.go
  • plugins/semanticcache/main.go

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@akshaydeo akshaydeo marked this pull request as ready for review January 11, 2026 16:05
@akshaydeo akshaydeo force-pushed the 01-11-fixes_race_condition_in_semanticcache branch from c766a8a to 90b4e04 Compare January 11, 2026 16:13
@akshaydeo akshaydeo merged commit 1c8fe47 into v1.4.0 Jan 11, 2026
2 of 4 checks passed
@akshaydeo akshaydeo deleted the 01-11-fixes_race_condition_in_semanticcache branch January 11, 2026 16:13
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