feat(collab): agent delegation handshake + sponsor_contact_id + cascades (D1)#2048
feat(collab): agent delegation handshake + sponsor_contact_id + cascades (D1)#2048hognek wants to merge 3 commits into
Conversation
…des (D1) Implement cross-user agent delegation (milestone D1 of jaylfc#2012): - Add sponsor_contact_id column to agent_registry (schema + migration + store API) - Delegation-request envelope dispatch in peer inbox (kind=delegation_request) - Policy gate with per-project auto_approve_delegation knob (default OFF) - Sponsored scope tiers: {a2a_send, a2a_receive, project_tasks, canvas_read, registry_feeds_read} - Hard-deny files_write and decisions_write at scope validation - Decisions card integration: collab_delegation_gate kind triggers invite mint on approve - Revoke cascade: contact/membership change → revoke sponsored identities → unassign tasks - 3-level kill-switch: per-agent (existing), per-contact pause, per-instance panic - Invite metadata column with sponsor_contact_id propagation through redeem flow - Project store helpers: is_project_member(), get/set_project_setting() Fixes: jaylfc#2019
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: 26 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 (9)
✨ 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}
| reply = "delegation denied" | ||
| elif completed: | ||
| reply = "delegation approved - the task has been assigned" | ||
| else: |
There was a problem hiding this comment.
CRITICAL: reply is unbound on the approved path — regression from this PR's edit.
The original code had if not approved: ... elif completed: ... else: ... but this change deleted the if not approved: and elif completed: conditions, collapsing them into a bare else:. Now reply = f"The delegation to {to_agent} was denied." is only assigned in the denied branch, while await _route_answer_to_agent(decision, reply) at line 420 always runs. On any approved delegation this raises UnboundLocalError/NameError, breaking the existing #161 gated-delegation approval flow (not just the new collab gate).
| else: | |
| if not approved: | |
| reply = "delegation denied" | |
| elif completed: | |
| reply = "delegation approved - the task has been assigned" | |
| else: | |
| reply = "delegation approved, but assigning the task failed - please retry" |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| return {"status": "error", "error": "registry not available"} | ||
|
|
||
| # Find all active agents sponsored by this contact. | ||
| sponsored = await registry.list_by_sponsor(contact_id, status="active") |
There was a problem hiding this comment.
WARNING: project_id is documented as restricting revocation to a single project, but the actual agent revocation ignores it.
list_by_sponsor(contact_id, status="active") returns every sponsored agent for the contact regardless of project, so set_status(..., "revoked") revokes ALL of the contact's sponsored identities across every project. Only the later task-unassign step honors project_id. The docstring says "When project_id is provided, only revoke tokens bound to that project (membership-revoke)" — the behavior contradicts the contract and causes over-revocation. Filter the sponsorees by project when project_id is set.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| contact_id = decision_metadata.get("contact_id", "") | ||
| agent_slug = decision_metadata.get("agent_slug", "") | ||
| display_name = decision_metadata.get("display_name", "") | ||
| granted_scopes = decision_metadata.get("granted_scopes", []) |
There was a problem hiding this comment.
WARNING: complete_delegation_approval mints the invite directly from stored granted_scopes without re-applying validate_delegation_scopes or binding to the authorized project.
The stored granted_scopes come from the original decision metadata. If those ever contain files_write/decisions_write they are minted verbatim — the hard-denylist is only enforced at request time (process_delegation_request), not at approval-completion time. Re-run validate_delegation_scopes(...) here and refuse/ strip any deny-scoped invite, and assert project_id matches the project the decision was created for, so a tampered or legacy decision can't mint an over-privileged invite.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| Reversible by clearing the flag. | ||
| """ | ||
| # Set a flag that the peer routes check. | ||
| request.app.state._peer_disabled = True |
There was a problem hiding this comment.
SUGGESTION: The per-instance panic flag has no re-enable path.
kill_switch_per_instance sets request.app.state._peer_disabled = True and the docstring claims "Reversible by clearing the flag", but no function clears it (and nothing resets it on restart). Operators have no supported way to resume peer traffic after a panic. Add an explicit resume/clear function or document that a restart is required.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| try: | ||
| from tinyagentos.projects.a2a import ensure_a2a_channel | ||
|
|
||
| if project_id: |
There was a problem hiding this comment.
SUGGESTION: A2A audit line is only emitted when project_id is set.
The "Collaboration with ... has been revoked" system message is guarded by if project_id:, so a contact-wide revoke (project_id=None) leaves no in-channel audit trail of which agents/tasks were revoked. Consider posting the summary to a default/admin channel or logging it even when project_id is absent.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previously Reported Issues — Now ResolvedAll 3 outstanding findings from the prior incremental review (at
Previous Review Summaries (2 snapshots, latest commit cccdaf9)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit cccdaf9)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Previously Reported Issues — Now ResolvedThe 5 findings from the prior review (at
Files Reviewed (2 changed since prior review)
Fix these issues in Kilo Cloud Previous review (commit dc2caa1)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (9 files)
Reviewed by hy3:free · Input: 35.9K · Output: 1.7K · Cached: 106K |
CRITICAL: Restore reply assignment in _apply_delegation_grant (regression from collapsed if/elif/else) WARNING: cascade_sponsor_revoke now filters agents by project_id via project_store.is_project_member for membership-scoped revoke WARNING: complete_delegation_approval re-applies scope denylist (validate_delegation_scopes) before minting invite SUGGESTION: Add kill_switch_reenable() for per-instance panic recovery SUGGESTION: A2A audit line emitted for contact-wide revoke across all projects the contact is a member of
| # approval. The denylist is the authoritative gate; the stored scopes are | ||
| # the human-approved set, but the code must never mint tokens for | ||
| # hard-denied scopes regardless. | ||
| safe_scopes, re_denied = validate_delegation_scopes(granted_scopes) |
There was a problem hiding this comment.
WARNING: Scope denylist is re-applied here (good), but project-membership is still NOT re-validated at approval time. This is the remaining half of the earlier complete_delegation_approval finding (#2): process_delegation_request checked is_project_member(contact_id, ..., member_kind="human") at request time, but a Decisions card can be approved much later. If the contact was removed from project_id (or the decision metadata was tampered) between request and approval, this still mints a live project invite for a project the sponsor no longer belongs to. Re-run is_project_member(project_id, contact_id, member_kind="human") here and refuse when it fails.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| for proj in (memberships or []): | ||
| pid = proj.get("id") or proj.get("project_id") | ||
| if not pid: | ||
| continue |
There was a problem hiding this comment.
WARNING: list_projects_for_member is called only inside a bare except AttributeError: memberships = []. This method is not defined anywhere in this PR (the new project_store helpers are is_project_member, get/set_project_setting), so the except AttributeError will trigger on every contact-wide revoke and silently produce no audit messages at all — defeating the entire point of this new branch. Either define list_projects_for_member on ProjectStore or drop the silent fallback so the missing-method bug surfaces loudly instead of hiding a missing audit trail for a security-relevant revocation.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| except Exception: | ||
| logger.warning( | ||
| "collab delegation completion failed for decision %s", | ||
| decision.get("id"), exc_info=True, |
There was a problem hiding this comment.
SUGGESTION: When complete_delegation_approval raises, this except Exception logs but routes no answer back to the agent. The decision answer was already persisted as approve, so the remote contact/agent is left permanently hanging with no notification that sponsor minting failed. Route a failure message (like the else branch at line 1316) inside the except so the caller learns the approval did not complete.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
WARNING: Re-validate project membership at approval time in complete_delegation_approval (fail-closed runtime check per design spec 5) WARNING: Remove fragile list_projects_for_member call (method does not exist on project_store). Simplify A2A audit to project-scoped only. SUGGESTION: Route error answer back when complete_delegation_approval raises, so the remote contact receives feedback on approval failure.
1 participant
Task: t_b72392f4. Fixes #2019.
Summary
Implement cross-user agent delegation (milestone D1 of EPIC #2012).
Changes
Files changed
Tests
311 tests pass across: test_collab_d1_delegation, test_agent_registry, test_project_store, test_invite_store, test_contacts_peer, test_auth_middleware, test_auth_requests_store