Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2f4f96557
ℹ️ 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".
| if (diffResult.exitCode === 0) { | ||
| return; |
There was a problem hiding this comment.
Retry failed pushes before declaring success
When backup.git.push is true and a job commits successfully but git push fails, the row is marked failed and then picked up by retry. On that retry the content is already in local HEAD, so this zero-diff branch returns before the push block and processJob records success even though the remote still does not have the backup commit. Please still attempt the configured push when there is no staged diff but pushing is enabled, or otherwise track the post-commit push state separately.
Useful? React with 👍 / 👎.
| backupStatus: true, | ||
| updatedAt: true, | ||
| }, | ||
| where: inArray(schema.memories.backupStatus, ["pending", "failed"]), |
There was a problem hiding this comment.
Requeue stale queued backups after restart
Because addMemory persists queued immediately after enqueue returns, a process crash or restart before the in-process worker finishes leaves that memory permanently in queued. Startup retry only selects pending and failed, so those stranded jobs are never retried and their markdown may never be committed. Since queued work is only process-local, include stale/any queued rows in the startup retry path or avoid persisting queued before durable processing begins.
Useful? React with 👍 / 👎.
| if (activeMemoryIds.has(job.memoryId)) { | ||
| return { backupStatus: "queued" }; |
There was a problem hiding this comment.
Preserve follow-up backup work for active memories
When another write for the same memory is enqueued while a previous backup is still active, this branch reports queued but drops the new job and its content paths. If that second write lands after the first backup has already committed or while it is just updating SQLite, the working tree can remain modified with no pending job and the memory can still end in success. Please coalesce the new paths into the active job or enqueue a follow-up run instead of discarding it.
Useful? React with 👍 / 👎.
| await updateBackupStatus({ | ||
| memoryId: job.memoryId, | ||
| backupStatus: "failed", | ||
| lastBackupAt: null, |
There was a problem hiding this comment.
Keep the last successful backup timestamp on failures
For a memory that was backed up successfully before, a later failed backup attempt overwrites last_backup_at with null, even though that field represents the last successful backup time. This loses useful state for any follow-up backup such as a highlight update or a retry after a previous success. The failure update should preserve the existing lastBackupAt and only update the error/status.
Useful? React with 👍 / 👎.
| config, | ||
| db: connection.db, | ||
| backupQueue: createNoopMemoryBackupQueue(), | ||
| backupQueue: getMemoryBackupQueue(config), |
There was a problem hiding this comment.
Start retries before a memory POST is handled
The new retry scan is only reached through getMemoryBackupQueue, and repo-wide search shows the only production call site is this POST path. That means existing pending/failed backups are not retried on server startup or while the user only browses existing memories; they wait until someone creates another memory. The startup retry should be initialized from a server startup/runtime path, not only from the add-memory request path.
Useful? React with 👍 / 👎.
Summary
Queue States
pending: markdown content exists and backup is eligible for queueing or retry.queued: work has been accepted by the in-process queue.success: git runner completed and metadata recordedlast_backup_at.failed: git runner or queue processing failed;last_backup_errorstores the diagnostic.disabled: configured git backup is off.Backup Status Source Of Truth
src/server/backup/status.tsexportsBACKUP_STATUSESandBackupStatus. SQLite schema constraints insrc/server/db/schema.tsnow derive from that shared constant.Git Commands
The runner uses
projectPathas cwd and clears inherited repository-scoped Git env before invoking nested git commands:git add -- <store-relative paths>git diff --cached --quiet -- <store-relative paths>git commit -m <commitMessageTemplate> -- <store-relative paths>git push <remote> HEAD:<branch>only whenbackup.git.pushis trueTest Strategy
tests/server/backup/git-backup.test.tscreates temporary git repositories and bare remotes to prove that only store files are staged, commits are created, disabled push leaves the remote untouched, failures preserve memory content/metadata, and startup retry does not duplicate active work.Validation
npm test -- tests/server/backup/git-backup.test.tsfailed before implementation because backup queue/runner exports were missing.npm test -- tests/server/backup/git-backup.test.ts-> pass (5 tests).npm run typecheck-> pass.npm test -- tests/server/backup/git-backup.test.ts tests/server/memories/add-memory.test.ts tests/server/db/schema.test.ts tests/server/routes/api-memories.test.ts-> pass (35 tests).npm test-> pass (16 files, 102 tests).env GIT_DIR=.tmp/gitdir GIT_WORK_TREE=. npm test-> pass (16 files, 102 tests; reproduces pre-push hook env).npm run build-> pass.git --git-dir=.tmp/gitdir --work-tree=. diff --check-> pass.typecheck,test, andbuild-> pass.bunis not on PATH in this workspace, so npm-equivalent scripts were used. E2E was not run because this task does not expose backup status UI.Linear: https://linear.app/haunted-fail/issue/TRA-9/task-08-git-backup-queue-workflow