fix(consent): request-consent skips caps with a pending Decision#1432
Conversation
The lazy first-use path re-fires request-consent on every denied capability access until the user answers. Without skipping caps that already have an unanswered app_grant Decision for the app, each call piled up a duplicate pending consent card. Now scan the user's pending app_grant Decisions for this app and exclude those caps too.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe request-consent route now ignores capabilities that already have pending app_grant decisions for the same app. A new async test covers repeated requests for the same capability and a later request for a different capability. ChangesApp consent request deduplication
Estimated review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Note Your trial team has used its Gitar budget, so automatic reviews are paused. Upgrade now to unlock full capacity. Comment "Gitar review" to trigger a review manually. Code Review ✅ ApprovedPrevents redundant consent card generation by filtering capabilities with pending app_grant decisions during the request process. All associated tests passed with no remaining issues. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Important Your trial ends in 2 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| pending: list[str] = [] | ||
| for c in caps: | ||
| if c in FREE_CAPS or c in decided or c in pending: | ||
| if c in FREE_CAPS or c in decided or c in awaiting or c in pending: |
There was a problem hiding this comment.
WARNING: Race window — two concurrent request-consent calls for the same app can both pass the awaiting check and each create a duplicate pending Decision.
The new scan at lines 150-154 closes the lazy re-fire gap (sequential calls from the broker) but does not close the concurrent-call gap: between await store.list(...) (line 151) and await store.create(...) (line 165) there is no lock, so two simultaneous first-time requests for the same app + cap will both observe an empty awaiting set and both produce a card. The PR description focuses on the lazy re-fire scenario, so this is a future-risk worth a follow-up (e.g. a UNIQUE constraint on (user_id, app_id, capability, status='pending'), or a single SQL upsert keyed on a pending app_grant).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| # so skip them or it would pile up duplicate pending consent cards. | ||
| store = request.app.state.decision_store | ||
| awaiting: set[str] = set() | ||
| for d in await store.list(status="pending", user_id=user.user_id): |
There was a problem hiding this comment.
SUGGESTION: Loading every pending decision for the user (up to the 500-row limit in DecisionStore.list) on every request-consent call is wasteful when the user has many non-app-grant pending decisions.
Filtering by kind == "app_grant" and app_id == ... is currently done in Python after a full table scan + JSON decode. If app_grant decisions ever become a meaningful fraction of the inbox this will dominate the request path. A small SQL filter (e.g. WHERE status='pending' AND json_extract(metadata, '$.kind') = 'app_grant' AND json_extract(metadata, '$.app_id') = ?) would push the predicates into SQLite and avoid the Python-side loop and decode.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 56.7K · Output: 6.2K · Cached: 322.5K |
1 participant
Gap
request-consent(#1430) de-dupes against granted/denied caps but not against caps that already have an unanswered app_grant Decision. The lazy first-use path (approved, timing = BOTH) re-fires on every denied capability access until the user answers, so repeated calls would pile up duplicate pending consent cards for the same app + cap.Fix
Before building the consent card, scan the user's pending app_grant Decisions for this app and exclude those capabilities (alongside the existing free-cap and already-decided filtering). Hardens the primitive before its callers (install flow + broker lazy-escalation) wire in during the live session.
Test
Second call for the same cap before answering returns
{decision: null, pending: []}; after answering, a request for a different cap still raises its own card. 34 related tests pass.Summary by CodeRabbit