feat(agents): canvas_read/canvas_write scopes (#1800 slice 1)#1824
Conversation
Add canvas_read and canvas_write to VALID_SCOPES and _ALLOWED_SCOPES. Canvas scopes require an explicit project_id at approval time, matching the project_tasks guard. Approver cannot widen scopes beyond what the agent requested. Docs-Reviewed: scope vocab additions per design doc lead-agent-identity-and-canvas-access.md
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 (1)
📝 WalkthroughWalkthroughCanvas read/write scopes are added to agent authorization. Approval now requires a project for canvas and project-task grants, while the desktop consent UI generalizes project-bound scope handling and route tests cover these approval rules. ChangesCanvas scope approval
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ConsentActions
participant AgentAuthRequests
participant AgentRegistry
ConsentActions->>AgentAuthRequests: Submit scopes and project_id
AgentAuthRequests->>AgentRegistry: Mint agent with validated scopes
AgentAuthRequests-->>ConsentActions: Return approval result or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| _PROJECT_SCOPES = {"project_tasks"} | _CANVAS_SCOPES | ||
|
|
||
| # project_tasks and the canvas scopes bind the token to a specific project | ||
| # and add a membership row, so require the human to pick that project |
There was a problem hiding this comment.
WARNING: Comment overstates current behavior — canvas scopes do NOT add a membership row.
This comment claims "project_tasks and the canvas scopes ... add a membership row", but the actual membership + a2a channel join at line 381 only triggers for "project_tasks" in body.granted_scopes. A canvas_read/canvas_write approval therefore gets a project-bound token and grant rows, but the agent is never added as a project member and never joins the a2a channel. Either extend the membership condition at line 381 to include canvas scopes, or correct this comment so it does not document behavior that does not exist yet.
| raise HTTPException( | ||
| status_code=400, | ||
| detail="project_id is required when granting project_tasks", | ||
| detail=f"project_id is required when granting {missing}", |
There was a problem hiding this comment.
SUGGESTION: The 400 detail renders a raw Python list repr (e.g. project_id is required when granting ['canvas_read', 'canvas_write']). Prefer a human-readable join.
| detail=f"project_id is required when granting {missing}", | |
| detail=f"project_id is required when granting: {', '.join(missing)}", |
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Incremental Change Notes
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous Review Summaries (3 snapshots, latest commit 297fd18)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 297fd18)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Incremental Change Notes
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 9874328)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Incremental Change Notes
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit d5ff3c1)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (4 files)
Reviewed by hy3:free · Input: 40.5K · Output: 6.1K · Cached: 169.7K |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/routes/agent_auth_requests.py (1)
44-63: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicated scope allowlist across two files.
VALID_SCOPESand_ALLOWED_SCOPESare identical 11-entry frozensets maintained independently; a future scope change to only one risks the two enforcement points silently diverging.
tinyagentos/routes/agent_auth_requests.py#L44-L63: extract this frozenset (with its per-scope rationale comments) into a shared module-level constant.tinyagentos/routes/agent_registry.py#L89-L96: import the shared constant here instead of redefining_ALLOWED_SCOPES.🤖 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 `@tinyagentos/routes/agent_auth_requests.py` around lines 44 - 63, The scope allowlist is duplicated across the authentication and registry routes; centralize it to prevent divergence. In tinyagentos/routes/agent_auth_requests.py lines 44-63, move the commented frozenset into a shared module-level constant and update local references to use it. In tinyagentos/routes/agent_registry.py lines 89-96, import that shared constant and remove the local _ALLOWED_SCOPES definition.
🧹 Nitpick comments (1)
tinyagentos/routes/agent_auth_requests.py (1)
318-320: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the already-computed
grantedset.
granted = set(body.granted_scopes)is computed at line 296; lines 318 and 320 recompute the same set instead of reusing it.♻️ Proposed fix
- needs_project = bool(set(body.granted_scopes) & _PROJECT_SCOPES) + needs_project = bool(granted & _PROJECT_SCOPES) if needs_project and body.project_id is None: - missing = sorted(set(body.granted_scopes) & _PROJECT_SCOPES) + missing = sorted(granted & _PROJECT_SCOPES)🤖 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 `@tinyagentos/routes/agent_auth_requests.py` around lines 318 - 320, Reuse the existing granted set in the project-scope validation within the relevant request handler: derive needs_project and missing from granted instead of recomputing set(body.granted_scopes).
🤖 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/components/ConsentActions.tsx`:
- Line 189: Update the incomplete label text near the ConsentActions component
to use a complete, clear phrase, such as “Project” or “Grant project access for
this request,” instead of ending with “for.”
- Around line 20-32: Use SCOPE_LABELS when rendering consent scopes in the
scope-list component, including the DecisionsApp.tsx consent display, so each
known scope ID shows its human-readable label instead of the raw identifier;
retain a sensible fallback for unmapped scopes.
In `@tinyagentos/routes/agent_auth_requests.py`:
- Around line 307-324: Update the project membership and A2A synchronization
logic for project-bound grants to include agents approved with canvas_read or
canvas_write, not only project_tasks. Reuse _CANVAS_SCOPES and preserve the
existing project_id binding; ensure canvas_write approvals receive the
membership data needed for can_edit_canvas.
---
Outside diff comments:
In `@tinyagentos/routes/agent_auth_requests.py`:
- Around line 44-63: The scope allowlist is duplicated across the authentication
and registry routes; centralize it to prevent divergence. In
tinyagentos/routes/agent_auth_requests.py lines 44-63, move the commented
frozenset into a shared module-level constant and update local references to use
it. In tinyagentos/routes/agent_registry.py lines 89-96, import that shared
constant and remove the local _ALLOWED_SCOPES definition.
---
Nitpick comments:
In `@tinyagentos/routes/agent_auth_requests.py`:
- Around line 318-320: Reuse the existing granted set in the project-scope
validation within the relevant request handler: derive needs_project and missing
from granted instead of recomputing set(body.granted_scopes).
🪄 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: 7fba18c4-d12a-4116-810f-8075e8da438f
📒 Files selected for processing (4)
desktop/src/components/ConsentActions.tsxtests/test_routes_agent_auth_requests.pytinyagentos/routes/agent_auth_requests.pytinyagentos/routes/agent_registry.py
| const SCOPE_LABELS: Record<string, string> = { | ||
| memory_read: "Read agent memory", | ||
| memory_write: "Write agent memory", | ||
| a2a_send: "Send messages to other agents", | ||
| a2a_receive: "Receive messages from other agents", | ||
| files_read: "Read files", | ||
| files_write: "Write files", | ||
| tools_execute: "Execute tools", | ||
| registry_feeds_read: "Read registry feeds", | ||
| project_tasks: "Read and manage project tasks", | ||
| canvas_read: "Read project canvas", | ||
| canvas_write: "Write to project canvas", | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'SCOPE_LABELS' desktop/src -C 3Repository: jaylfc/taOS
Length of output: 717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ConsentActions outline =="
ast-grep outline desktop/src/components/ConsentActions.tsx --view expanded || true
echo
echo "== Search for SCOPE_LABELS / scope rendering =="
rg -n 'SCOPE_LABELS|project_tasks|canvas_read|canvas_write|memory_read|memory_write|a2a_send|a2a_receive|files_read|files_write|tools_execute|registry_feeds_read' desktop/src -C 2
echo
echo "== Relevant nearby lines in ConsentActions.tsx =="
sed -n '1,260p' desktop/src/components/ConsentActions.tsxRepository: jaylfc/taOS
Length of output: 23071
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for user-facing scope rendering =="
rg -n 'requested_scopes|granted_scopes|SCOPE_LABELS|Read agent memory|Write agent memory|Send messages to other agents|Read and manage project tasks|Grant project access for|memory_read|project_tasks|canvas_read|canvas_write' desktop/src -C 2
echo
echo "== ConsentActions render section =="
sed -n '260,360p' desktop/src/components/ConsentActions.tsx
echo
echo "== Related notification / decision components =="
for f in desktop/src/components/NotificationCentre.tsx desktop/src/components/NotificationToast.tsx desktop/src/apps/DecisionsApp.tsx; do
echo "--- $f ---"
sed -n '1,260p' "$f"
echo
doneRepository: jaylfc/taOS
Length of output: 43885
Wire SCOPE_LABELS into the consent scope list. SCOPE_LABELS is still unused here, and the only scope display found renders raw IDs in desktop/src/apps/DecisionsApp.tsx, so approvers still won’t see the human-readable consent text.
🤖 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/components/ConsentActions.tsx` around lines 20 - 32, Use
SCOPE_LABELS when rendering consent scopes in the scope-list component,
including the DecisionsApp.tsx consent display, so each known scope ID shows its
human-readable label instead of the raw identifier; retain a sensible fallback
for unmapped scopes.
| className="block text-[11px] text-shell-text-secondary mb-1" | ||
| > | ||
| Grant project_tasks for project | ||
| Grant project access for |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Incomplete label text.
"Grant project access for" reads as a truncated sentence with no object. Consider "Project" or "Grant project access for this request".
✏️ Proposed fix
- Grant project access for
+ Project📝 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.
| Grant project access for | |
| Project |
🤖 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/components/ConsentActions.tsx` at line 189, Update the incomplete
label text near the ConsentActions component to use a complete, clear phrase,
such as “Project” or “Grant project access for this request,” instead of ending
with “for.”
| _CANVAS_SCOPES = {"canvas_read", "canvas_write"} | ||
| _PROJECT_SCOPES = {"project_tasks"} | _CANVAS_SCOPES | ||
|
|
||
| # project_tasks and the canvas scopes bind the token to a specific project | ||
| # and add a membership row, so require the human to pick that project | ||
| # explicitly in the consent card. Never fall back to the agent-supplied | ||
| # project_id for these grants: POST /api/agents/auth-requests is | ||
| # unauthenticated, so the request could name any existing project the | ||
| # operator never validated. Other scopes keep the fallback so global | ||
| # tokens still work. Checked before any registration so a rejected approval | ||
| # never leaves an orphaned agent. | ||
| needs_project = bool(set(body.granted_scopes) & _PROJECT_SCOPES) | ||
| if needs_project and body.project_id is None: | ||
| missing = sorted(set(body.granted_scopes) & _PROJECT_SCOPES) | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail="project_id is required when granting project_tasks", | ||
| detail=f"project_id is required when granting {missing}", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'project_tasks.*granted_scopes|granted_scopes.*project_tasks' tinyagentos/routes/agent_auth_requests.py -B5 -A15Repository: jaylfc/taOS
Length of output: 1233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of the approval flow.
sed -n '300,410p' tinyagentos/routes/agent_auth_requests.py
printf '\n---\n'
# Find canvas scope handling elsewhere in the repo.
rg -n '"canvas_read"|"canvas_write"|canvas_scopes|canvas' tinyagentos -g '!**/__pycache__/**'Repository: jaylfc/taOS
Length of output: 30822
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the project canvas routes for how canvas_read/canvas_write are enforced.
sed -n '1,260p' tinyagentos/routes/project_canvas.py
printf '\n---\n'
# Inspect project canvas store permission checks around can_edit_canvas and member rows.
sed -n '1,320p' tinyagentos/projects/canvas/store.py
printf '\n---\n'
# Find where project membership is used as the authoritative signal for canvas access.
rg -n 'can_edit_canvas|project_members|add_member\(|a2a channel|membership' tinyagentos/routes tinyagentos/projects -g '!**/__pycache__/**'Repository: jaylfc/taOS
Length of output: 22308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the agent-facing canvas tools to see whether they depend on project membership.
sed -n '1,260p' tinyagentos/projects/canvas/mcp_tools.py
printf '\n---\n'
# Inspect project store schema and add_member behavior for the member row fields.
sed -n '1,280p' tinyagentos/projects/project_store.pyRepository: jaylfc/taOS
Length of output: 17998
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect project a2a membership resolution and channel sync behavior.
sed -n '1,260p' tinyagentos/projects/a2a.py
printf '\n---\n'
# Show the scope definitions at the top of agent_auth_requests.py.
sed -n '1,90p' tinyagentos/routes/agent_auth_requests.pyRepository: jaylfc/taOS
Length of output: 12101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any other place where canvas scopes or project_tasks map to member permissions.
rg -n 'canvas_read|canvas_write|can_edit_canvas|project_tasks' tinyagentos -g '!**/__pycache__/**'
printf '\n---\n'
# Inspect any approval or grant logic around project member permissions.
rg -n 'add_member\(|can_edit_canvas =|set_canvas_permission|ensure_a2a_channel' tinyagentos/routes tinyagentos/projects -g '!**/__pycache__/**'Repository: jaylfc/taOS
Length of output: 7827
Canvas scopes should join the project membership sync
canvas_read/canvas_write are project-bound now, but this block still only adds project_tasks agents to project_members and the A2A channel. Canvas-only approvals won’t appear in Members, and canvas_write agents can’t get can_edit_canvas without a membership row. Include _CANVAS_SCOPES here, or split read vs write handling.
🤖 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 `@tinyagentos/routes/agent_auth_requests.py` around lines 307 - 324, Update the
project membership and A2A synchronization logic for project-bound grants to
include agents approved with canvas_read or canvas_write, not only
project_tasks. Reuse _CANVAS_SCOPES and preserve the existing project_id
binding; ensure canvas_write approvals receive the membership data needed for
can_edit_canvas.
The scope label map was added but never rendered (no scope-label site in this component; that belongs to the Members UI in a later slice), which failed the strict SPA build with an unused-declaration error. Remove it; the canvas scopes already drive the project picker via needsProject. Docs-Reviewed: build fix only, no behaviour change, scope vocab per lead-agent-identity-and-canvas-access.md
Adversarial review flagged that the project-binding guard only checked 'is None', so an empty or whitespace project_id passed through and minted a token with a falsy project_id that downstream truthy checks treat as unbound, opening a cross-project canvas write. Fail closed on None, empty, and whitespace, and add a regression test for both blank forms. Docs-Reviewed: defense-in-depth on the canvas scope project binding per lead-agent-identity-and-canvas-access.md
|
Folded an adversarial-review finding before this lands: the project-binding guard on the approve path only rejected a null project_id, so an empty or whitespace value slipped through and minted a token with a falsy project_id that a downstream truthy check would treat as unbound, allowing cross-project canvas access. Now fails closed on null, empty, and whitespace, with a regression test covering both blank forms. Other review notes: the write-implies-read invariant is enforced in the slice 3 gating (the D3 matrix explicitly tests that canvas_write alone cannot read), and the duplicate scope list is already guarded by test_allowed_scopes_matches_canonical_valid_scopes, so both are out of scope here. |
| # Reject None, "", and whitespace-only: a blank project_id is not a real | ||
| # binding, and a downstream truthy check would treat it as unbound, so an | ||
| # empty string must fail closed exactly like a missing one. | ||
| if needs_project and not (body.project_id and body.project_id.strip()): |
There was a problem hiding this comment.
SUGGESTION: Guard strips for emptiness but the bound project_id is stored unstripped
This rejects whitespace-only ids (e.g. " ") but accepts a non-empty, whitespace-padded id such as " prj-canvas-1 ", since both body.project_id and body.project_id.strip() are truthy. Downstream, effective_project (line 360) is assigned body.project_id verbatim and used unchanged for the minted token, grant rows, and the membership sync. A padded value therefore won't match the real project id and can bind the agent to a non-existent project. Consider normalizing (.strip()) body.project_id once before it is stored, or tightening this check to also reject padded ids.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Slice 1 renamed the picker label from 'Grant project_tasks for project' to 'Grant project access for' (the label now covers canvas scopes too), but the component test still looked the picker up by the old label, so three cases failed in the SPA build vitest run. Point them at the new label. Test-only change. Docs-Reviewed: test label sync for the project-picker rename in slice 1 per lead-agent-identity-and-canvas-access.md
1 participant
Slice 1 of the #1800 lead-agent identity epic.
Changes:
DO NOT MERGE — held for the full-set review.
Summary by CodeRabbit
canvas_readandcanvas_write.