feat(invite): project_invites store + mint/list/revoke routes (#1780 S1)#1855
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
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? |
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ) | ||
| row = await cursor.fetchone() | ||
| pending_count = row[0] if row else 0 | ||
| if pending_count >= _PENDING_CAP: |
There was a problem hiding this comment.
SUGGESTION: Pending-cap and list queries key on project_id = ?, but project_id is now nullable (commit 2). Invites minted with project_id=None will never be counted toward _PENDING_CAP (SQLite treats NULL = NULL as false) and won't be returned by list_for_project(None). If agent-centric invites with no project are ever minted through this store, the 10-pending cap becomes a no-op for them and they silently disappear from project listings.
Consider keying the cap count on a coalesced/project-scoped condition and deciding explicitly whether NULL-project invites should participate in the cap.
| await self._db.commit() | ||
|
|
||
| def _generate_invite_id(self) -> str: | ||
| return f"{secrets.randbelow(1_000_000):06d}" |
There was a problem hiding this comment.
SUGGESTION: invite_id is a 6-digit random number with no uniqueness/collision retry before the INSERT. With a PRIMARY KEY on invite_id, a collision raises an unhandled IntegrityError, surfacing as a 500. The probability is low but non-zero, especially under high mint volume or after the id space is exhausted.
Consider looping a few times to regenerate invite_id on IntegrityError (or widening the id space).
| raise InviteExpiredError("invite has expired") | ||
|
|
||
| attempts = row["redeem_attempts"] or 0 | ||
| if attempts >= _MAX_ATTEMPTS: |
There was a problem hiding this comment.
SUGGESTION: Once redeem_attempts reaches _MAX_ATTEMPTS, a subsequent call (even with the correct PIN) falls through line 229 and raises InvitePinError("invalid invite id or pin"). The invite is actually locked/expired, so this error message is misleading to callers and masks the real state.
Consider checking expired/locked status first (or raising a distinct InviteLockedError/InviteExpiredError) so the response reflects that the invite is exhausted rather than that the PIN was wrong.
| await s.close() | ||
|
|
||
|
|
||
| def _make_scopes(*extra): |
There was a problem hiding this comment.
SUGGESTION: _make_scopes is defined but never used anywhere in this test module. It appears to be dead test code (the store itself already forces project_tasks in mint).
Remove it to avoid confusion, or wire it into a test if it was intended to assert the allowed-scope ordering.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 files in incremental diff)
Fix these issues in Kilo Cloud Previous Review Summary (commit a1524b2)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a1524b2)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (7 files)
Reviewed by hy3:free · Input: 39.9K · Output: 3.8K · Cached: 193.8K |
An Agents-app invite registers an agent with no project bound; SQLite cannot drop NOT NULL via ALTER later, so relax it before the store ships. Docs-Reviewed: invite mint/list/revoke are admin-only project-management routes (UI-consumed, not part of the agent-JWT surface); the agent-facing contract is the S2 redeem route + connection bundle, specified in docs/design/external-agent-project-invite.md
jaylfc
force-pushed
the
feat/invite-s1-store
branch
from
a1524b2 to
a58179e
Compare
1 participant
Slice S1 of the external-agent invite feature.
Implements the ProjectInviteStore with mint/get/list_for_project/revoke/redeem, the admin-gated POST/GET/DELETE routes under /api/projects/{project_id}/invites, and wires the store + router into app.py.
Tests cover: 6-digit invite_id + 4-digit PIN, project_tasks always included, 10-pending cap -> 429, wrong-pin attempts + 5th invalidates, correct-pin single-use (2nd redeem raises), expired invite, revoke, list omits pin_hash, and a boot-migration test (seed a project_invites table missing redeemed_request_id, assert init() boots).