Fold tsk-2bkd56: lead card-edit via project_tasks_update, not scope collapse (PR 2240 remainder) - #2244
Conversation
📝 WalkthroughWalkthroughLead agents now receive the project-scoped ChangesProject task updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LeadAgent
participant TaskRoute
participant TaskStore
LeadAgent->>TaskRoute: PATCH task with project_tasks_update
TaskRoute->>TaskRoute: Check ownership and editable fields
TaskRoute->>TaskStore: Persist title, body, labels, or priority
TaskStore-->>LeadAgent: Updated task
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
|
|
nemotron-ultra-kilo review VERDICT: Approved with minor concerns — scope separation is solid, tests are thorough, but field validation has a potential None-bypass and missing status-field rejection test.
Automated first-pass review by the nemotron-ultra-kilo lane. The lead still reviews before merge. |
|
Reviewed against fold card tsk-b6ugu5: on-spec. Scope stays project_tasks_update (route untouched), lead seed gains the grant with a re-seed re-assert test, and the regression pin is real - I ran it green on this branch (62 passed), then collapsed the PATCH scope to project_tasks locally exactly as PR 2240 did and the pin went RED. deleted-symbols-gate red was the intentional removal of test_patch_task_is_session_only (it asserted the pre-#2184 session-only design; the new pin supersedes it) - waived via Removes-Intentionally trailer now in the body. Kilo red is today's rate-limit noise. Merging when the gate re-runs green. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/test_routes_projects_agent_tasks.py`:
- Line 241: Update the assignment from _mint_agent in the affected test to bind
the unused first return value as _cid instead of cid, while preserving token
usage and the existing test behavior.
In `@tinyagentos/routes/projects.py`:
- Around line 818-824: Update the agent-field validation in
tinyagentos/routes/projects.py lines 818-824 to inspect
payload.model_fields_set, rejecting every supplied field outside
_AGENT_EDITABLE_FIELDS even when its value is null. Add regression cases in
tests/test_routes_projects_agent_tasks.py lines 206-234 asserting 403 responses
for status, element_id, and a payload containing body with status set to null.
🪄 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: 1752e04e-f007-4ce0-adab-ae027f3b49d4
📒 Files selected for processing (7)
CHANGELOG.mddocs/agent-coordination.mdtests/test_agent_internal_mint.pytests/test_routes_projects_agent_tasks.pytinyagentos/auth_middleware.pytinyagentos/routes/agent_registry.pytinyagentos/routes/projects.py
| its cards -> 403.""" | ||
| pid = await _new_project(ctx, "alpha") | ||
| tid = await _new_task(ctx, pid) | ||
| cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks_update",)) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused cid binding.
The test does not use cid. Rename it to _cid to satisfy Ruff RUF059.
Proposed fix
- cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks_update",))
+ _cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks_update",))📝 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.
| cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks_update",)) | |
| _cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks_update",)) |
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 241-241: Unpacked variable cid is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 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 `@tests/test_routes_projects_agent_tasks.py` at line 241, Update the assignment
from _mint_agent in the affected test to bind the unused first return value as
_cid instead of cid, while preserving token usage and the existing test
behavior.
Source: Linters/SAST tools
| if is_agent: | ||
| for f in payload.model_fields: | ||
| if f not in _AGENT_EDITABLE_FIELDS and getattr(payload, f) is not None: | ||
| return JSONResponse( | ||
| {"error": f"field {f!r} is not editable by agents"}, | ||
| status_code=400, | ||
| status_code=403, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate protected agent fields by request presence. The current non-null check permits an agent to include a protected field with null in an otherwise valid PATCH request. Use payload.model_fields_set and add regression coverage for this boundary.
tinyagentos/routes/projects.py#L818-L824: reject every supplied field outside_AGENT_EDITABLE_FIELDS, including fields supplied asnull.tests/test_routes_projects_agent_tasks.py#L206-L234: add 403 cases forstatus,element_id, and{"body": "...", "status": null}.
📍 Affects 2 files
tinyagentos/routes/projects.py#L818-L824(this comment)tests/test_routes_projects_agent_tasks.py#L206-L234
🤖 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/projects.py` around lines 818 - 824, Update the
agent-field validation in tinyagentos/routes/projects.py lines 818-824 to
inspect payload.model_fields_set, rejecting every supplied field outside
_AGENT_EDITABLE_FIELDS even when its value is null. Add regression cases in
tests/test_routes_projects_agent_tasks.py lines 206-234 asserting 403 responses
for status, element_id, and a payload containing body with status set to null.
Code Review SummaryStatus: No New Issues Found | Recommendation: Merge Overview
Files Reviewed (7 files)
Note: The existing nemotron-ultra-kilo review (comment ID 5159735065) already raised the Reviewed by step-3.7-flash · Input: 95.7K · Output: 39.2K · Cached: 1.2M |
1 participant
CARD TITLE (intent, not commit subject): Fold tsk-2bkd56: lead card-edit via project_tasks_update, not scope collapse (PR 2240 remainder)
Autonomous build of board card tsk-b6ugu5.
Files:
pyproject.toml | 2 +-
tests/test_agent_internal_mint.py | 38 ++++++-
tests/test_routes_projects_agent_tasks.py | 164 ++++++++++++++++++++++++++++--
tinyagentos/init.py | 2 +-
tinyagentos/auth_middleware.py | 2 +-
tinyagentos/routes/agent_registry.py | 31 +++---
tinyagentos/routes/projects.py | 25 +++--
9 files changed, 225 insertions(+), 118 deletions(-)
Removes-Intentionally: tests/test_routes_projects_agent_tasks.py:TestAgentCanDriveOwnBoard.test_patch_task_is_session_only
Summary by CodeRabbit
New Features
Bug Fixes
Documentation