Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ed8e4ab15
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ba564e740
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aef3f41227
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67f96b44c0
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dd2e9bc17
ℹ️ 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".
0dd2e9b to
d7f62b8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7f62b8aff
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 861c21342a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae01737684
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b27446a135
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c93db00f3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe93799348
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74ba57b9dc
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 629bcf730d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df456e4965
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1fb0efb77
ℹ️ 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".
| await PRAutomation.deleteOne(guard); | ||
| return; | ||
| } | ||
| await PRAutomation.replaceOne(guard, previous); |
There was a problem hiding this comment.
When an enable writes its pending epoch, a concurrent user stop can successfully change that same record to stopped without rotating the epoch; if account deletion then makes the enable fail its second liveness check, this replaceOne still matches and restores the older active preimage. After deletion is cancelled, automation can therefore run despite the successful stop. Prevent mutations of pending records or include a mutation/version guard so rollback cannot overwrite intervening state changes.
AGENTS.md reference: AGENTS.md:L44-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 07f9283. The rollback restores only the fields the enable wrote, and only while each still holds the value it left, so a concurrent stop or trust change survives. Covered by the two 'keeps a ... while a rejected enable rolls back' tests.
codegraph-librechat
Bot
added
🗺️ Data Models & DB
e1fb0ef to
a3b296c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3b296cd4a
ℹ️ 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".
| /** | ||
| * Conversations also leave through the retention index, which never runs | ||
| * `deleteConvos`, so liveness is checked here rather than trusted to a cascade. |
There was a problem hiding this comment.
When MongoDB's TTL index deletes a conversation and no later PR delivery arrives—for example, an idle automation on a quiet pull request—this claim-time liveness check never runs. A repo-wide search shows that only deleteConvos, account deletion, and refuseClaim remove PRAutomation records, while its schema has no TTL or periodic orphan sweep, so repository bindings and trusted-bot metadata can remain indefinitely after conversation retention. Fresh evidence beyond the earlier late-enable cleanup thread is the explicitly bypassing retention-index path documented here; add a durable expiration or orphan-cleanup mechanism rather than relying on a future claim.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 26b3477. The TTL index skips deleteConvos, so reapPRAutomations now runs hourly on the elected replica, reuses the claim-time liveness check, and removes records whose conversation, ancestor or owner is gone. Covered by prAutomation.spec.ts, 'reaping records the retention index left behind'.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07f9283b51
ℹ️ 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".
| .lean<Array<{ _id: Types.ObjectId; user: string; conversationId: string }>>(); | ||
| let removed = 0; | ||
| for (const record of page) { | ||
| const key = { userId: String(record.user), conversationId: record.conversationId }; |
There was a problem hiding this comment.
The new hourly caller runs this reaper under runAsSystem, but this key discards the record's tenantId, so checkLiveness begins its aggregation with only { user, conversationId } and $limit: 1. Because the conversation index permits that pair in multiple tenants, the reaper can select another tenant's expired conversation and delete a live automation, or select another tenant's live conversation and retain an orphan. Project tenantId on the page and pass it through every liveness lookup instead of deriving the tenant from an arbitrary first match.
AGENTS.md reference: AGENTS.md:L187-L190
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f74617d. The reaper now passes each record's tenantId into the first match of the liveness lookup, so a record is judged only by its own tenant's conversation. Covered by 'judges each record by its own tenant when the same chat exists in two', which fails without the change.
| * idempotent and race-free. It excludes `tenantId` for the same reason as | ||
| * `ToolFavorite`: user ObjectIds are globally unique. | ||
| */ | ||
| prAutomationSchema.index({ user: 1, conversationId: 1 }, { unique: true }); |
There was a problem hiding this comment.
When the same global user has the same conversationId in two tenants—a layout explicitly allowed by the conversation schema's { conversationId, user, tenantId } unique index—the tenant-scoped upsert in the second tenant cannot see the first record but this global unique index still rejects its insert with E11000. Consequently, one tenant's automation prevents that user from enabling the distinct conversation in another tenant; make the uniqueness key match the tenant-scoped identity.
AGENTS.md reference: AGENTS.md:L187-L190
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f74617d. The unique index is now { user, tenantId, conversationId }, matching the conversation's own identity; the collection is new in this PR, so no migration. Covered by 'keeps one record per tenant for the same user and conversation id', which fails on the old index.
| await connectDb(); | ||
| logger.info(`Worker ${process.pid}: Connected to MongoDB`); | ||
| startCodeEnvironmentLifecycleReconciler({ mongoose }); | ||
| startPRAutomationReaper({ reapPRAutomations }); |
There was a problem hiding this comment.
In the inspected experimental.js entrypoint, CLUSTER_WORKERS defaults to four while the backend:experimental script does not enable Redis. Every worker executes this line, and LeaderElection.isLeader() returns true unconditionally when Redis is disabled, so all four workers perform the complete reaper scan both at startup and every hour. Since each record adds owner and lineage database queries, larger installations receive the same expensive scan four times concurrently; use the existing primary-to-worker assignment mechanism or another single-worker guard for non-Redis clustered mode.
AGENTS.md reference: AGENTS.md:L187-L190
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f74617d. createClusteredPRAutomationReaper starts the reaper only in the worker the primary assigns the file-retention sweep to when Redis is off, and in every worker under leader election when it is on; experimental.js only wires it. Covered by automation.spec.ts. The same gap in the code-environment reconciler is tracked at berry-13#382.
07f9283 to
f74617d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f74617d303
ℹ️ 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 (!isGone(await checkLiveness(key, { tenantId: record.tenantId }))) { | ||
| continue; | ||
| } | ||
| const result = await PRAutomation.deleteOne({ _id: record._id, ...keyFilter(key) }); |
There was a problem hiding this comment.
If checkLiveness reports conversation_gone but the conversation is recreated with the same ID before this delete—saveConvo can upsert a missing conversation—and the user enables automation again, this predicate still matches the original _id and removes the newly valid automation. Carry a conversation-incarnation or equivalent version fence through the liveness check and delete so stale reaper decisions cannot erase a record renewed after the check.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e236d81. The reaper deletes on { _id, version }, and every write including a no-op enable moves the version, so a record renewed after the check is kept. Covered by 'keeps a record renewed between its check and its delete'.
| * failed: the next claim removes any record whose owner no longer exists. A retry on an | ||
| * account that was already gone has committed nothing, so its failure still throws. */ | ||
| try { | ||
| await prAutomation.deletePRAutomations(userId); |
There was a problem hiding this comment.
When account deletion runs for an authenticated tenant user, the ambient tenant context causes applyTenantIsolation to add that tenant to this cleanup's deleteMany, even though the User deletion is global and automation records can exist for the same user in multiple tenants. The controller therefore reports successful account deletion while records in the user's other tenants remain until a later reaper happens to remove them; run this account-wide cascade under system scope while retaining the user predicate.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e236d81. The account-wide cascade runs under system scope with the user predicate. Covered by 'removes the records in every tenant when the deletion runs in one'.
| { ...RESTARTABLE, ...otherRepository }, | ||
| reset({ ...revive, ...binding, trustedBots: [] }), | ||
| ); |
There was a problem hiding this comment.
If an enable has passed its initial checks but has not reached this write, a concurrent user_stopped call can successfully set state: 'stopped' and return, after which this RESTARTABLE predicate matches that newly stopped record and resets it to idle, clearing the stop code. Fresh evidence beyond the earlier stop-fence fix is this enable-side revival path, which lets automation continue after the user's stop succeeded; condition revival on the record version observed when enable began, or rotate and compare a mutation generation so only a stop predating the enable can be restarted.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e236d81. The record now carries a version that every write increments; enable makes one write fenced on the version it read, so a stop that lands in between makes the enable return conflict instead of reviving the record. Covered by 'keeps a user stop that lands between an enable reading the record and writing it'.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 483af3b3f6
ℹ️ 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 (!isGone(await checkLiveness(key, { tenantId: record.tenantId }))) { | ||
| return 0; |
There was a problem hiding this comment.
When retention deletes a conversation and saveConvo upserts the same { user, tenantId, conversationId } before the hourly pass, this check treats the replacement as the original conversation and keeps the old repository, trust, and epoch claimable by delayed events. Fresh evidence after the earlier renewal thread is that the new automation version guard is never reached when the replacement appears live, and recreating the conversation alone does not bump that version; persist and validate a conversation incarnation such as its document _id.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c188e21. Records now store conversationRef, the _id of the conversation they were enabled for, and claims and the reaper treat a different document as gone, so a conversation saved again under the same id does not inherit the old record. Covered by the 'a conversation deleted and saved again under the same id' tests. Subagent parent links are still keyed by conversation id; that boundary is tracked at berry-13#383.
| /** Only after the delete committed, so a failed delete keeps the user's automation. | ||
| * A claim refuses any record whose conversation is gone and removes it, so a cleanup | ||
| * that still fails after its retry converges on the next claim. */ | ||
| await retryCascadeOperation(() => prAutomation.deletePRAutomations(user, waveIds)).catch( |
There was a problem hiding this comment.
If saveConvo recreates the same conversation ID after Conversation.deleteMany returns but before this cleanup runs, a concurrent enable can successfully create a record for the replacement and this identity-only delete then removes that new record. Fence cleanup to the automation snapshot or conversation incarnation associated with the deleted document so a completed enable for the replacement survives.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c188e21 and d680b14. The cascade deletes only records of the conversation documents it removed, and the retry for an already deleted root reaps through the liveness check, so a record enabled for a conversation saved again meanwhile is kept. Covered by 'keeps a record enabled for the conversation saved again before the cleanup'.
Adds the endpoints.agents.prAutomation config block (off by default) and a per-conversation record that holds the automation state, round counter, trust level and per-repository bot allowlist. Round claims are a single conditional update so racing or duplicate deliveries cannot exceed the cap.
Round completions now settle only the round that owns them, a user stop is unconditional and requires a stop code, claimed heads are remembered so a delayed delivery of an earlier head cannot start a round, approved bots are cleared when the conversation is bound to a different repository, the approved bot limit is configurable, and the record is removed with its conversation and with its user. Import order is fixed for the static check.
Binding a conversation to another pull request starts a fresh run and writes the repository and number in one conditional update, and each claim carries a run id so a completion from a replaced run cannot settle the new one. Conversation and account deletion stop the record first and remove it only after the delete commits, so a failed delete keeps the stored state.
A round claim now names the pull request it is for and applies only while the record is still bound to it, so an event delivered after a rebind cannot spend the new pull request's budget. Enabling a record applies its reset and binding in one conditional write per case, so a stopped record stays stopped when the new binding is rejected. The unique index is ensured before the first write, because automatic index creation can be disabled.
Enabling a record no longer clears a conversation or account deletion fence, and a deletion fence replaces an earlier user stop. A bot approval names the repository it was authorized for and applies only while the record is still bound to it. A round claim removes the record instead of starting work when its conversation was removed by the retention index or is past its retention date.
An event-driven stop names the pull request it is for and applies only while the record is still bound to it, while a user stop stays unconditional. A bot removal names its repository like an approval does. Disabling no longer removes a deletion fence. The time window opens in the same atomic write as the first claimed round. Account deletion removes the records before the account so a failed cleanup is retried instead of being swallowed.
…e Enables A deletion now writes a tombstone for a conversation that has no record yet, so an enable already in flight cannot create an idle record under it. When the delete settles, the fence is released: a record whose conversation is gone is removed, and one whose conversation survived becomes a restartable stop with the deletion_aborted code. Account deletion removes the records only after the user delete committed. A claim for a subagent thread also requires its parent and root conversations to be active. The schema rejects a fractional pull request number.
…letions Conversation and account deletion no longer write fences or tombstones. They remove records after the delete commits, in bounded batches, so a deletion of a conversation that never used the feature writes nothing. A round claim now checks, before and after its atomic write, that the owner has no account deletion in progress and that the conversation and its ancestors are active; a claim that loses that race hands its round back and never starts work. The auth cache is invalidated before the fallible cleanup, and the redundant user index is dropped.
…neage in One Query
…rue Enable Preimage
…s Only When Current
Conversations also leave through the TTL index on expiredAt, which never runs deleteConvos, so an idle automation record outlived its conversation until a delivery happened to arrive. reapPRAutomations pages through the records and removes those whose conversation, ancestor or owner is gone, reusing the claim-time liveness check, and the elected replica runs it hourly from both server entries.
A rejected enable restored the whole preimage under an epoch-only guard, so a stop or a trust change that landed after its write, which keeps the epoch, was discarded when an account deletion started before the enable's second check. The rollback now restores only the fields the attempt wrote, and only while each still holds the value the attempt left; otherwise the record stays pending and unclaimable.
The reaper runs as system, so its liveness check matched the conversation by user and id alone and could judge a record by another tenant's copy. It now passes each record's tenant into the first match. Records are unique per user, tenant and conversation, like the conversations they belong to. In the clustered server without Redis every worker counts as the leader, so the reaper now runs only in the worker the primary assigns the file-retention sweep to.
Writes that acted on an earlier read had nothing to compare against, so a user stop that landed between an enable's read and its write was revived, and a reaper or claim delete could remove a record renewed after its liveness check. Every write now increments version. Enable reads the record once and makes one write fenced on the version it read, returning a conflict when it moved; its rollback and commit fence on the version it wrote, which replaces the per-field rollback. With that read in hand, an enable also narrows a stored trust level above the current ceiling. A rejected claim still undoes itself by its run id, so a trust or bot change made meanwhile cannot strand the round. The reaper, the claim refusal and the enable's own cleanup delete only the version they judged. Account deletion removes automation records in every tenant.
A claim whose post-write liveness read threw had already spent a round and recorded the head, so a retry saw a record that was not active and nothing ever released it. The claim now undoes itself by its run id before rethrowing, through the same release a rejected claim uses.
The reaper ran on a fixed hour. endpoints.agents.prAutomation.reaperIntervalMinutes sets it, from 5 to 1440 minutes with a default of 60, and both server entries pass the loaded base config to the reaper so the yaml value is the one that runs.
The reaper checked each record of a page one after another, two reads per record, so a large collection could take longer than the interval. It now runs up to eight liveness checks at once within a page, keeping the page order and the cursor.
A conversation deleted and saved again under the same id looked like the original, so its old record stayed claimable and a cascade delete could remove a record enabled for the replacement. Records now store conversationRef, the _id of the conversation they were enabled for. Claims and the reaper treat a different document as gone, an enable on an earlier conversation's record starts over for the new one, and the conversation cascade deletes only records of the documents it removed.
A record deleted and enabled again starts its version count over, so a claim refusal or an enable cleanup that named only the version could remove the new record. Those deletes, the enable commit and its rollback now name the document _id they saw along with the version.
…cord An enable fenced its write on the version it read but not on the record, so a record disabled and enabled again to the same version was overwritten and left pending. Its write now names the record it read. The deletion retry that cleans up after a root a previous attempt already removed deleted by conversation id alone; it now reaps those records through the liveness check, so a record of a conversation saved again under the same id is kept.
483af3b to
d680b14
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d680b1490a
ℹ️ 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".
| const conversationRef = String(conversation._id); | ||
| if (scope?.conversationRef != null && scope.conversationRef !== conversationRef) { | ||
| return judged('conversation_gone'); |
There was a problem hiding this comment.
For a subagent whose root is TTL-deleted and then recreated with the same conversationId before the next claim or reaper pass, the replacement root is accepted as the original ancestor because the walk validates ancestor IDs while the incarnation check covers only the leaf conversation. The old descendant automation therefore remains claimable even though its controlling root disappeared; persist and verify document identities for the full lineage, not just conversationRef for the leaf.
AGENTS.md reference: AGENTS.md:L45-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 95a82fb. Records now also store lineageRefs, the _id of each ancestor of a subagent thread, and the liveness check compares every ancestor it walks, so a root or parent deleted and saved again under the same id makes the record gone; an enable on such a record starts over. Covered by the three 'deleted and saved again' subagent tests. This also closes berry-13#383.
A subagent thread's ancestors were matched by conversation id, so a root or parent deleted and saved again under the same id still controlled the old record. Records now store lineageRefs, the _id of each ancestor in walk order, and the liveness check treats a different document at any position as gone. An enable on a record whose ancestors changed starts over.
Summary
There is no place to store per-conversation state for an automation that fixes CI failures and review comments on a conversation's pull request, and no administrator setting that controls it. This adds the
endpoints.agents.prAutomationconfig block (absent by default,enabled: false) and aPRAutomationrecord per user, tenant and conversation holding the state, stop code, round counter, time window, last head, trust level and per-repository approved bots. Nothing reads the config yet, so no behavior changes for users; the executor, routes and UI follow in the stack (berry-13#380).Starting a round is one conditional update over state, round cap, time window and head, so racing deliveries cannot pass the cap. Every write increments a
versionfield, and writes that act on an earlier read (enable, its rollback and commit, the claim refusal, the reaper) are fenced on it, so a stop or renewal that lands in between is never undone. A leader-gated reaper removes records whose conversation, ancestor or owner is gone, including conversations the TTL index deleted; it runs everyreaperIntervalMinutes(default 60) in one worker, a few checks at a time. Records are bound to the conversation document they were enabled for and to each ancestor document of a subagent thread, so a conversation deleted and saved again under the same id does not inherit an old record. Related to #16792. Fixes berry-13#383.Type of change
Testing
prAutomation.spec.ts(data-schemas, mongodb-memory-server): round cap and concurrent claims, version fencing against a racing stop and a renewal during reaping, conversations and ancestors deleted and saved again under the same id, rollback keeping concurrent stops and trust changes, two-tenant records and reaping, claim release on a failed check, bounded parallel reaping.user.methods.spec.ts: account deletion removes records in every tenant.service.spec.ts: a yamlreaperIntervalMinutesreaches the app config.automation.spec.ts(packages/api): cursor paging, leader gating, single-worker assignment without Redis, and the configured interval.prAutomation.spec.ts(data-provider): config defaults and bounds.reviewctl precheck(static checks plus jest for api, packages/api, data-provider, data-schemas) passes; tsc clean for every touched workspace.Screenshots / recordings
No user-facing change.
Risk / compatibility
New collection with a unique
{ user, tenantId, conversationId }index; nothing writes to it until the executor ships. The config block is optional, so existinglibrechat.yamlfiles are unaffected. The record stores no credentials. Tested against MongoDB only, not DocumentDB.Checklist