fix(memory): handle missing analyze deps without permanently failing job#25676
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 370bf4a3de
ℹ️ 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".
| { jobId: job.id, conversationId }, | ||
| "Skipping job: analysis deps not yet initialized; will retrigger", | ||
| ); | ||
| return; |
There was a problem hiding this comment.
Requeue when analysis deps are unavailable
Do not return success here when getAnalysisDeps() is null: in runMemoryJobsOnce, any handler that resolves is immediately completeMemoryJob(...), so this path permanently drops the conversation_analyze job without running analysis. The assumption that it will “retrigger” is not guaranteed, because auto-analysis enqueueing happens on later batch/idle/lifecycle events; conversations with a pre-existing queued job during startup and no subsequent activity will never be analyzed.
Useful? React with 👍 / 👎.
|
Addressed in #26266 — conversation_analyze job now throws |
Summary
Fix gap from plan review for auto-analyze-loop.md.
The previous handler threw a plain Error when the analyze-deps singleton was unpopulated, expecting the worker to reschedule. In reality, plain Errors are classified as fatal and immediately marked failed. This change returns gracefully so the next upstream trigger re-enqueues cleanly during slow daemon startup.