fix(brainbar): make digest and ack truthful - #635
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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: a34a4c90ee
ℹ️ 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".
| } | ||
|
|
||
| do { | ||
| guard try database.subscription(agentID: agentID) != nil else { |
There was a problem hiding this comment.
Reject rows left behind by false acknowledgements
On an upgraded database, this guard still accepts every agent row that the old false-ack path created because subscription() checks brainbar_agents, while the former acknowledge() inserted that row before advancing its cursors. REPORT.md:29 confirms that production already contains acknowledged agent rows with zero subscription tags, so subsequent acknowledgements for those IDs will still return acked and advance their cursors. Validate actual brainbar_subscriptions membership and handle legacy zero-tag rows before acknowledging.
AGENTS.md reference: AGENTS.md:L6-L7
Useful? React with 👍 / 👎.
|
VERDICT: APPROVE |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d0cbddf7-ad10-45fb-87d1-b963759474c5) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_68a35fcf-cc57-477c-b68a-f9dc302184e0) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2573b0fe-dc25-405d-9c88-b978dce9fede) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
What broke and how it was found
brain_digest silently truncated input to 500 characters while returning success, and brain_ack acknowledged unknown inboxes by auto-creating cursor state. Direct socket reproduction measured the digest cutoff and the false-ack row creation.
Evidence
Review
This branch was pair-reviewed by a separate agent under the taste gate: smallest correct solution, behavioural tests, and no speculative machinery.
Operational note
Enrichment and drain are currently disabled with launchctl to prevent provenance-tag decay. Re-enable deliberately after the provenance-column ownership question is settled.
Note
Medium Risk
Changes MCP persistence and inbox cursor semantics; callers that relied on ack succeeding for unsubscribed agents will now see errors, while digest storage size grows for long inputs.
Overview
brain_digestno longer truncates stored text to 500 characters plus...; the Swift digest path now persists the full input (with an optional title prefix), matching the schema’s much larger limit and stopping silent loss on long tool payloads.brain_ackon the socket server now requires an existing BrainBar subscription before callingacknowledge. Unsubscribed agents get a JSON-RPC tool error (No BrainBar subscription for agent: …) and no new agent/cursor row from the old auto-create path.Adds
REPORT.mddocumenting measured digest cutoff, stub counts, recovery notes, and false-ack reproduction. NewMCPRouterTestsandSocketIntegrationTestslock in full-length digest storage and ack rejection for unknown agents.Reviewed by Cursor Bugbot for commit 1628b63. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix
brain_digesttruncation andbrain_ackfalse acknowledgment in BrainBarbrain_digestnow stores the full input content (with optional title prepended) instead of truncating to 500 characters with an ellipsis, fixing data loss on long inputs.brain_acknow checks for an existing subscription before acknowledging; if no subscription exists for the agent, it returns an error and does not implicitly create a record.MCPRouterTests.swiftasserts full content is stored for a 701-character input.SocketIntegrationTests.swiftassertsbrain_ackrejects unknown agents and leaves the database unchanged.brain_ackcalls that previously silently succeeded for unregistered agents now return a JSON-RPC error response.📊 Macroscope summarized 1628b63. 3 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.