feat(invite): ProjectMembers invite-external-agent UI (#1780 S4)#1857
Conversation
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? |
|
Warning Review limit reached
Next review available in: 35 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an external-agent invitation dialog to Projects. It supports configurable scopes, lead assignment, approval mode, check-in intervals, invite result display, pending invite countdowns, revocation, and integration with the project Members view. ChangesExternal agent invites
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectMembers
participant InviteAgentDialog
participant ProjectsInvitesAPI
ProjectMembers->>InviteAgentDialog: open external-agent invite dialog
InviteAgentDialog->>ProjectsInvitesAPI: fetch pending invites
InviteAgentDialog->>ProjectsInvitesAPI: POST invite configuration
ProjectsInvitesAPI-->>InviteAgentDialog: return invite URL and one-time PIN
InviteAgentDialog-->>ProjectMembers: invoke onMinted
InviteAgentDialog->>ProjectsInvitesAPI: DELETE selected invite
ProjectsInvitesAPI-->>InviteAgentDialog: return refreshed pending invites
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| type="number" | ||
| min={0} | ||
| value={intervalSecs} | ||
| onChange={(e) => setIntervalSecs(Number(e.target.value) || 0)} |
There was a problem hiding this comment.
SUGGESTION: Invalid / empty interval input silently coerces to 0
Number(e.target.value) || 0 collapses empty string, NaN, and negative/0 values all to 0, so a user who clears the field or types garbage will POST check_interval_secs: 0, which is almost certainly not a valid check-in interval. Consider validating the parsed value (e.g. clamp to a positive minimum, or keep the previous value when the new one is NaN).
| onChange={(e) => setIntervalSecs(Number(e.target.value) || 0)} | |
| onChange={(e) => { const v = Number(e.target.value); setIntervalSecs(Number.isFinite(v) && v >= 0 ? v : 0); }} |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| }, [result]); | ||
|
|
||
| return createPortal( | ||
| <div |
There was a problem hiding this comment.
WARNING: Dialog cannot be dismissed via the backdrop or Escape key
The role="dialog" overlay has no onClick to close when the backdrop is clicked, and there is no keydown/Escape handler. Users can only close via the Cancel/Done buttons. This is a minor accessibility/UX gap and inconsistent with typical modals in app dialogs.
| <div | |
| role="dialog" | |
| aria-modal="true" | |
| aria-label="Invite external agent" | |
| className="fixed inset-0 bg-black/50 flex items-center justify-center p-4" | |
| onMouseDown={(e) => { if (e.target === e.currentTarget) onClose(); }} | |
| onKeyDown={(e) => { if (e.key === "Escape") onClose(); }} |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if (s.disabled) continue; | ||
| if (scopes[s.value]) out.add(s.value); | ||
| } | ||
| if (isLead) out.add("lead"); |
There was a problem hiding this comment.
SUGGESTION: Lead designation via scope vs. exclusive project Lead selector may conflict
This dialog adds "lead" to the posted scopes, but ProjectMembers already has a separate "exclusive per project" Lead <select> (lines 367-369) enforcing a single-lead invariant. Having two independent ways to designate a lead (MemberRow lead selector + this invite-scope toggle) risks confusing/inconsistent semantics and could let a redeemed external agent claim lead while the UI's exclusive selector shows someone else. Worth confirming the backend treats invite-scope lead the same way, or documenting why they coexist.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const refreshPending = () => { | ||
| fetch(`/api/projects/${projectId}/invites`, { credentials: "include" }) | ||
| .then((r) => (r.ok ? r.json() : [])) | ||
| .then((rows) => setPending(Array.isArray(rows) ? (rows as InviteRecord[]) : [])) |
There was a problem hiding this comment.
SUGGESTION: Non-OK GET for pending invites silently treated as empty list
On a non-ok response r.json() is not attempted and [] is used, so a 401/500 simply hides the error with no surfaced feedback (unlike onSubmit/onRevoke which set error). If the invites endpoint fails auth, the user sees "No pending invites." with no indication something went wrong. Consider surfacing error here too, or at least distinguishing empty from failed.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files)
Incremental note: since the previous review (commit Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit 9f57c0a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 9f57c0a)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous reviewStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files)
Reviewed by hy3:free · Input: 36.7K · Output: 2.6K · Cached: 185.2K |
| projectId={project.id} | ||
| onClose={() => setInviteOpen(false)} | ||
| onMinted={() => { | ||
| setInviteOpen(false); |
There was a problem hiding this comment.
WARNING: onMinted immediately closes the dialog, hiding the PIN/invite-URL result view
After a successful mint, InviteAgentDialog calls onMinted?.() (InviteAgentDialog.tsx:141) and then renders the result section that shows the PIN (marked "shown once"), the invite URL, and the copyable instruction (InviteAgentDialog.tsx:184-240). But this handler calls setInviteOpen(false), which unmounts the dialog instantly. The user never sees or can copy the PIN/URL.
Either remove setInviteOpen(false) from onMinted and let the dialog's "Done" button (which calls onClose) be the only closer, or move the onChanged() refresh into onClose. For example:
| setInviteOpen(false); | |
| onMinted={() => { | |
| onChanged(); | |
| }} |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/apps/ProjectsApp/InviteAgentDialog.tsx`:
- Around line 90-95: Update refreshPending so fetch or response failures no
longer call setPending with an empty array. Preserve the current pending invites
on failure and expose the failure through the dialog’s existing loading-error
state or mechanism, while retaining normal replacement of the list for
successful valid responses.
- Around line 340-367: Update the pending invite rendering in InviteAgentDialog
so status and revoke availability are derived from the computed remaining
countdown: when remaining is zero or less, display the expired status and
disable or remove the Revoke action; otherwise preserve the existing invite
status chip and revoke behavior. Use the existing remaining, statusChipClass,
and statusChipLabel flow.
In `@desktop/src/apps/ProjectsApp/ProjectMembers.tsx`:
- Around line 428-430: Update the onMinted handler in ProjectMembers so it only
triggers onChanged and does not call setInviteOpen(false), keeping
InviteAgentDialog mounted after minting. Preserve closing behavior through the
dialog’s existing Done action and onClose handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d27a52d-0bdd-4830-b095-c8cc7a88b5e9
📒 Files selected for processing (3)
desktop/src/apps/ProjectsApp/InviteAgentDialog.tsxdesktop/src/apps/ProjectsApp/ProjectMembers.tsxdesktop/src/apps/ProjectsApp/__tests__/InviteAgentDialog.test.tsx
| const refreshPending = () => { | ||
| fetch(`/api/projects/${projectId}/invites`, { credentials: "include" }) | ||
| .then((r) => (r.ok ? r.json() : [])) | ||
| .then((rows) => setPending(Array.isArray(rows) ? (rows as InviteRecord[]) : [])) | ||
| .catch(() => setPending([])); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Do not report fetch failures as “No pending invites.”
A transient or authorization failure clears known invites and renders a false empty state. Preserve the existing list and expose a loading error instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/ProjectsApp/InviteAgentDialog.tsx` around lines 90 - 95,
Update refreshPending so fetch or response failures no longer call setPending
with an empty array. Preserve the current pending invites on failure and expose
the failure through the dialog’s existing loading-error state or mechanism,
while retaining normal replacement of the list for successful valid responses.
| {pending.map((inv) => { | ||
| const remaining = Math.floor(inv.expires_ts) - now; | ||
| return ( | ||
| <li | ||
| key={inv.invite_id} | ||
| className="flex flex-col gap-1 text-xs bg-zinc-900/60 border border-zinc-800 rounded px-2 py-2 md:flex-row md:items-center md:justify-between md:gap-2" | ||
| > | ||
| <div className="min-w-0"> | ||
| <span className="font-mono">{inv.invite_id}</span> | ||
| <span className="text-zinc-500 ml-2">{inv.scopes.join(", ")}</span> | ||
| </div> | ||
| <div className="flex items-center gap-2"> | ||
| <span className="tabular-nums text-zinc-400" title="Time until expiry"> | ||
| {formatCountdown(remaining)} | ||
| </span> | ||
| <span | ||
| className={`px-1.5 py-0.5 rounded text-[10px] uppercase tracking-wide ${statusChipClass(inv.status)}`} | ||
| > | ||
| {statusChipLabel(inv.status, inv.redeemed_by)} | ||
| </span> | ||
| <button | ||
| type="button" | ||
| onClick={() => onRevoke(inv.invite_id)} | ||
| className="text-xs text-red-400 hover:underline" | ||
| aria-label={`Revoke ${inv.invite_id}`} | ||
| > | ||
| Revoke | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Derive the displayed status from the countdown.
When a pending invite reaches zero, the countdown says “expired,” but its chip remains “pending” and the revoke action stays available until another fetch occurs.
Proposed fix
const remaining = Math.floor(inv.expires_ts) - now;
+const displayedStatus =
+ remaining <= 0 && inv.status === "pending" ? "expired" : inv.status;
...
-className={`... ${statusChipClass(inv.status)}`}
+className={`... ${statusChipClass(displayedStatus)}`}
>
- {statusChipLabel(inv.status, inv.redeemed_by)}
+ {statusChipLabel(displayedStatus, inv.redeemed_by)}
</span>
-{/* revoke button */}
+{displayedStatus === "pending" && (
+ /* revoke button */
+)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {pending.map((inv) => { | |
| const remaining = Math.floor(inv.expires_ts) - now; | |
| return ( | |
| <li | |
| key={inv.invite_id} | |
| className="flex flex-col gap-1 text-xs bg-zinc-900/60 border border-zinc-800 rounded px-2 py-2 md:flex-row md:items-center md:justify-between md:gap-2" | |
| > | |
| <div className="min-w-0"> | |
| <span className="font-mono">{inv.invite_id}</span> | |
| <span className="text-zinc-500 ml-2">{inv.scopes.join(", ")}</span> | |
| </div> | |
| <div className="flex items-center gap-2"> | |
| <span className="tabular-nums text-zinc-400" title="Time until expiry"> | |
| {formatCountdown(remaining)} | |
| </span> | |
| <span | |
| className={`px-1.5 py-0.5 rounded text-[10px] uppercase tracking-wide ${statusChipClass(inv.status)}`} | |
| > | |
| {statusChipLabel(inv.status, inv.redeemed_by)} | |
| </span> | |
| <button | |
| type="button" | |
| onClick={() => onRevoke(inv.invite_id)} | |
| className="text-xs text-red-400 hover:underline" | |
| aria-label={`Revoke ${inv.invite_id}`} | |
| > | |
| Revoke | |
| </button> | |
| {pending.map((inv) => { | |
| const remaining = Math.floor(inv.expires_ts) - now; | |
| const displayedStatus = | |
| remaining <= 0 && inv.status === "pending" ? "expired" : inv.status; | |
| return ( | |
| <li | |
| key={inv.invite_id} | |
| className="flex flex-col gap-1 text-xs bg-zinc-900/60 border border-zinc-800 rounded px-2 py-2 md:flex-row md:items-center md:justify-between md:gap-2" | |
| > | |
| <div className="min-w-0"> | |
| <span className="font-mono">{inv.invite_id}</span> | |
| <span className="text-zinc-500 ml-2">{inv.scopes.join(", ")}</span> | |
| </div> | |
| <div className="flex items-center gap-2"> | |
| <span className="tabular-nums text-zinc-400" title="Time until expiry"> | |
| {formatCountdown(remaining)} | |
| </span> | |
| <span | |
| className={`px-1.5 py-0.5 rounded text-[10px] uppercase tracking-wide ${statusChipClass(displayedStatus)}`} | |
| > | |
| {statusChipLabel(displayedStatus, inv.redeemed_by)} | |
| </span> | |
| {displayedStatus === "pending" && ( | |
| <button | |
| type="button" | |
| onClick={() => onRevoke(inv.invite_id)} | |
| className="text-xs text-red-400 hover:underline" | |
| aria-label={`Revoke ${inv.invite_id}`} | |
| > | |
| Revoke | |
| </button> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/ProjectsApp/InviteAgentDialog.tsx` around lines 340 - 367,
Update the pending invite rendering in InviteAgentDialog so status and revoke
availability are derived from the computed remaining countdown: when remaining
is zero or less, display the expired status and disable or remove the Revoke
action; otherwise preserve the existing invite status chip and revoke behavior.
Use the existing remaining, statusChipClass, and statusChipLabel flow.
| onMinted={() => { | ||
| setInviteOpen(false); | ||
| onChanged(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the dialog open after minting so the one-time PIN remains visible.
InviteAgentDialog calls onMinted immediately after setting its result, so this handler unmounts the dialog before users can view or copy the URL, PIN, and instructions. The standalone result test uses a no-op callback and therefore misses this integration failure.
Proposed fix
onMinted={() => {
- setInviteOpen(false);
onChanged();
}}The existing Done action already closes the dialog through onClose.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onMinted={() => { | |
| setInviteOpen(false); | |
| onChanged(); | |
| onMinted={() => { | |
| onChanged(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@desktop/src/apps/ProjectsApp/ProjectMembers.tsx` around lines 428 - 430,
Update the onMinted handler in ProjectMembers so it only triggers onChanged and
does not call setInviteOpen(false), keeping InviteAgentDialog mounted after
minting. Preserve closing behavior through the dialog’s existing Done action and
onClose handler.
Docs-Reviewed: InviteAgentDialog.tsx is a sub-component of the existing Projects app, not a new desktop app; no README app-list change is warranted (the invite UX is covered by docs/design/external-agent-project-invite.md).
9f57c0a to
daa252b
Compare
…ents app (#1861) (#1867) * feat(agents): AssignAgentToProjectDialog to share an agent with a project (#1861) TDD: failing test first, then component. POSTs to /api/projects/{pid}/members/assign-agent with canonical_id, scopes (project_tasks always present), and is_lead when the Lead toggle is set. Docs-Reviewed: AssignAgentToProjectDialog.tsx is a sub-component of the existing Agents app, not a new desktop app (mirrors the InviteAgentDialog sub-component pattern from #1857). * feat(agents): invite + assign external agents from the Agents app (#1861) Add an admin-only 'Invite external agent' button that opens a project picker then the existing InviteAgentDialog, and a per-row 'Assign to project' action (admin/owner, active entries) that opens AssignAgentToProjectDialog. Docs-Reviewed: RegistryPanel changes and the new InviteExternalAgentPicker sub-component are part of the existing Agents app, not a new desktop app (mirrors #1857).
1 participant
ProjectMembers slice of the external-agent invite feature. Adds an "Invite external agent" button beside Add agent that opens InviteAgentDialog.
Dialog:
vitest covers all five required assertions. tsc --noEmit is clean.
Summary by CodeRabbit
New Features
Tests