tsk-gq43l6 [OPEN] Lists S1: backend stores+routes+scopes (#2140) - #2144
tsk-gq43l6 [OPEN] Lists S1: backend stores+routes+scopes (#2140)#2144jaylfc wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis change adds SQLite-backed project list entry persistence, wires the new stores into application startup and state, exports project store classes, and adds project-scoped ChangesProject lists
Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
|
nemotron-ultra-kilo review VERDICT: Code adds project lists feature with stores and auth scopes; has SQL injection risks, race conditions, missing constraints, and no tests.
Automated first-pass review by the nemotron-ultra-kilo lane. The lead still reviews before merge. |
PR Summary by QodoAdd project list stores and lists_* agent scopes
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
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 `@tinyagentos/projects/lists_store.py`:
- Line 8: Resolve the missing ProjectListsStore contract: in
tinyagentos/projects/lists_store.py:8-8, implement ProjectListsStore alongside
ProjectListEntriesStore or remove its usage; in
tinyagentos/projects/__init__.py:3-3, export only symbols actually defined by
lists_store; and in tinyagentos/app.py:401-401, construct only implemented
stores or use the implemented ProjectListsStore.
- Around line 49-53: Update add_entry() so the multiline SQL passed to
self._db.execute uses a properly closed triple-quoted string, preserving the
existing INSERT statement and bind parameters while eliminating parser errors.
- Around line 42-57: When creating an entry in the method containing the INSERT,
replace the omitted-position fallback of 0 with the result of the existing
_get_next_position() helper. Preserve explicitly supplied positions while
ensuring new entries append after the current list contents.
🪄 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: ce07306e-cbd2-46da-aa1a-f5e5ea17df85
📒 Files selected for processing (4)
tinyagentos/app.pytinyagentos/projects/__init__.pytinyagentos/projects/lists_store.pytinyagentos/routes/agent_auth_requests.py
| from tinyagentos.base_store import BaseStore | ||
| from tinyagentos.projects.ids import new_id | ||
|
|
||
| class ProjectListEntriesStore(BaseStore): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Implement or stop importing ProjectListsStore. The new module defines ProjectListEntriesStore only, but both package export and app startup import ProjectListsStore; after the syntax error is fixed, this raises ImportError and prevents application startup.
tinyagentos/projects/lists_store.py#L8-L8: add theProjectListsStoreimplementation expected by this PR.tinyagentos/projects/__init__.py#L3-L3: export only symbols actually provided bytinyagentos.projects.lists_store.tinyagentos/app.py#L401-L401: construct only implemented stores, or use the newly addedProjectListsStore.
📍 Affects 3 files
tinyagentos/projects/lists_store.py#L8-L8(this comment)tinyagentos/projects/__init__.py#L3-L3tinyagentos/app.py#L401-L401
🤖 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/projects/lists_store.py` at line 8, Resolve the missing
ProjectListsStore contract: in tinyagentos/projects/lists_store.py:8-8,
implement ProjectListsStore alongside ProjectListEntriesStore or remove its
usage; in tinyagentos/projects/__init__.py:3-3, export only symbols actually
defined by lists_store; and in tinyagentos/app.py:401-401, construct only
implemented stores or use the implemented ProjectListsStore.
| position: int | None = None, | ||
| ) -> dict: | ||
| entry_id = new_id("lle") | ||
| now = time.time() | ||
|
|
||
| # Store immutability: original_text cannot be changed after creation | ||
| # It is stored verbatim as the first written text | ||
| await self._db.execute( | ||
| "INSERT INTO project_list_entries ( | ||
| id, list_id, project_id, text, original_text, category, status, | ||
| done, author_kind, author_id, position, created_at, updated_at | ||
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", | ||
| ( | ||
| entry_id, list_id, project_id, text, original_text, | ||
| category, "new", 0, author_kind, author_id, | ||
| position if position is not None else 0, now, now, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Append entries at the next list position.
When position is omitted, every entry is stored at position 0; after any reorder, new entries can appear before existing entries. Use the existing _get_next_position() helper instead.
Proposed fix
) -> dict:
entry_id = new_id("lle")
now = time.time()
+ if position is None:
+ position = await self._get_next_position(project_id, list_id)
# Store immutability: original_text cannot be changed after creation
@@
- position if position is not None else 0, now, now,
+ position, now, now,📝 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.
| position: int | None = None, | |
| ) -> dict: | |
| entry_id = new_id("lle") | |
| now = time.time() | |
| # Store immutability: original_text cannot be changed after creation | |
| # It is stored verbatim as the first written text | |
| await self._db.execute( | |
| "INSERT INTO project_list_entries ( | |
| id, list_id, project_id, text, original_text, category, status, | |
| done, author_kind, author_id, position, created_at, updated_at | |
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", | |
| ( | |
| entry_id, list_id, project_id, text, original_text, | |
| category, "new", 0, author_kind, author_id, | |
| position if position is not None else 0, now, now, | |
| position: int | None = None, | |
| ) -> dict: | |
| entry_id = new_id("lle") | |
| now = time.time() | |
| if position is None: | |
| position = await self._get_next_position(project_id, list_id) | |
| # Store immutability: original_text cannot be changed after creation | |
| # It is stored verbatim as the first written text | |
| await self._db.execute( | |
| "INSERT INTO project_list_entries ( | |
| id, list_id, project_id, text, original_text, category, status, | |
| done, author_kind, author_id, position, created_at, updated_at | |
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", | |
| ( | |
| entry_id, list_id, project_id, text, original_text, | |
| category, "new", 0, author_kind, author_id, | |
| position, now, now, |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 50-50: missing closing quote in string literal
(invalid-syntax)
[warning] 51-51: Expected ,, found name
(invalid-syntax)
[warning] 53-53: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: missing closing quote in string literal
(invalid-syntax)
[warning] 54-54: Unexpected indentation
(invalid-syntax)
🤖 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/projects/lists_store.py` around lines 42 - 57, When creating an
entry in the method containing the INSERT, replace the omitted-position fallback
of 0 with the result of the existing _get_next_position() helper. Preserve
explicitly supplied positions while ensuring new entries append after the
current list contents.
| await self._db.execute( | ||
| "INSERT INTO project_list_entries ( | ||
| id, list_id, project_id, text, original_text, category, status, | ||
| done, author_kind, author_id, position, created_at, updated_at | ||
| ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ruff check tinyagentos/projects/lists_store.pyRepository: jaylfc/taOS
Length of output: 20372
Fix add_entry() so it can be parsed.
The current line 50 multi-line string uses a normal double-quoted string, which leaves the ruff/Python parser with multiple invalid-string and cascade parsing errors instead of a concise “missing closing quote” for just this SQL statement. Wrap the SQL in a triple-quoted string, or keep it on one line and pass the bind tuple as the second argument to execute().
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 49-59: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🪛 Ruff (0.15.21)
[warning] 50-50: missing closing quote in string literal
(invalid-syntax)
[warning] 51-51: Expected ,, found name
(invalid-syntax)
[warning] 53-53: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: Expected an expression or a ')'
(invalid-syntax)
[warning] 53-53: Got unexpected token ?
(invalid-syntax)
[warning] 53-53: missing closing quote in string literal
(invalid-syntax)
🤖 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/projects/lists_store.py` around lines 49 - 53, Update add_entry()
so the multiline SQL passed to self._db.execute uses a properly closed
triple-quoted string, preserving the existing INSERT statement and bind
parameters while eliminating parser errors.
Source: Linters/SAST tools
|
Reviewed. Not mergeable yet - three blockers, one of them fatal. 1. The module does not parse. 2. No routes. The card scope was stores + routes + scopes, and the diff touches 3. No tests. The card's acceptance criteria required them; the diff adds none. What is good, and should be kept as-is: the schema is thoughtful. Splitting the remainder so each piece is small enough to land cleanly:
Leaving this open for the fix-forward. |
Code Review by Qodo
1. _ALLOWED_SCOPES missing lists scopes
|
| # Lists access: idea capture with agent triage (statuses, checkboxes, agent tidy) | ||
| # Least-privilege read + write on the agent's OWN project only, bound by the | ||
| # token's project_id claim. | ||
| "lists_read", | ||
| "lists_write", |
There was a problem hiding this comment.
1. _allowed_scopes missing lists scopes 📜 Skill insight ≡ Correctness
lists_read and lists_write were added to VALID_SCOPES but not to _ALLOWED_SCOPES in agent_registry.py, so the two scope sets are no longer synchronized. This likely breaks the equality check/test and can prevent minting/granting the new scopes consistently.
Agent Prompt
## Issue description
The PR adds new scopes (`lists_read`, `lists_write`) to the consent-flow `VALID_SCOPES`, but `tinyagentos/routes/agent_registry.py`'s `_ALLOWED_SCOPES` is not updated, violating the requirement that these sets stay synchronized.
## Issue Context
A test asserts these scope sets are equal. If they drift, registry minting/validation paths may reject scopes that the consent flow considers valid.
## Fix Focus Areas
- tinyagentos/routes/agent_registry.py[87-100]
- tinyagentos/routes/agent_auth_requests.py[43-81]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| from tinyagentos.projects.lists_store import ProjectListsStore, ProjectListEntriesStore | ||
| project_store = ProjectStore(data_dir / "projects.db") | ||
| project_invite_store = ProjectInviteStore(data_dir / "project_invites.db") | ||
| project_lists_store = ProjectListsStore(data_dir / "project_lists.db") | ||
| project_list_entries_store = ProjectListEntriesStore(data_dir / "project_list_entries.db") |
There was a problem hiding this comment.
2. Missing projectlistsstore class 🐞 Bug ≡ Correctness
tinyagentos/app.py imports and instantiates ProjectListsStore, but tinyagentos/projects/lists_store.py defines only ProjectListEntriesStore, causing ImportError during backend startup/probe. This prevents the FastAPI app from initializing stores and serving requests.
Agent Prompt
## Issue description
`ProjectListsStore` is imported/instantiated during app setup but is not defined in `tinyagentos/projects/lists_store.py`, which will raise `ImportError` and crash startup.
## Issue Context
`tinyagentos/app.py` now wires both `ProjectListsStore` and `ProjectListEntriesStore` into `app.state` and calls `.init()` during lifespan.
## Fix Focus Areas
- tinyagentos/projects/lists_store.py[1-182]
- tinyagentos/app.py[401-405]
- tinyagentos/projects/__init__.py[1-16]
## Suggested fix
- Either implement a `ProjectListsStore` class (with its own `SCHEMA` for lists metadata and the API expected by routes), or stop importing/instantiating/exporting it until it exists.
- Ensure the store(s) you keep are initialized in lifespan and exposed via `app.state` consistently with how other stores are wired.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| _LISTS_SCOPES = {"lists_read", "lists_write"} | ||
| _PROJECT_SCOPES = { | ||
| "project_tasks", | ||
| "project_tasks_create", | ||
| "lists_read", | ||
| "lists_write", | ||
| } | _CANVAS_SCOPES | _FILES_SCOPES | _LISTS_SCOPES |
There was a problem hiding this comment.
3. Scope-set test will fail 🐞 Bug ☼ Reliability
_PROJECT_SCOPES now includes lists_read/lists_write, but tests/test_agent_scope_requests.py asserts an exact set without those entries, so CI will fail. This blocks verification of the PR until the expected scope set is updated.
Agent Prompt
## Issue description
The PR expands `_PROJECT_SCOPES` to include `lists_read` and `lists_write`, but the unit test still asserts the previous exact set contents.
## Issue Context
`tests/test_agent_scope_requests.py::test_project_scope_set_is_a_single_definition` compares `_PROJECT_SCOPES` to a hard-coded set.
## Fix Focus Areas
- tinyagentos/routes/agent_auth_requests.py[200-209]
- tests/test_agent_scope_requests.py[554-580]
## Suggested fix
- Update the expected set in the test to include `lists_read` and `lists_write`.
- Keep the alias identity assertions (`is`) intact so drift is still caught.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| async def _get_next_position(self, project_id: str, list_id: str | None = None) -> int: | ||
| async with self._db.execute( | ||
| "SELECT MAX(position) + 1 FROM project_list_entries WHERE project_id = ? AND list_id = ?", | ||
| (project_id, list_id or ""), | ||
| ) as cur: |
There was a problem hiding this comment.
4. Next position computed incorrectly 🐞 Bug ≡ Correctness
ProjectListEntriesStore._get_next_position converts list_id=None into an empty string, but list_id is stored as a real non-empty identifier, so the query will not match existing rows and will return 0. Any caller using this helper with list_id omitted will repeatedly assign position 0 and break ordering semantics.
Agent Prompt
## Issue description
`_get_next_position()` substitutes `None` list_id with `""`, which won’t match normal stored `list_id` values, causing the computed next position to reset to 0.
## Issue Context
Schema enforces `list_id TEXT NOT NULL`, and `add_entry()` inserts the provided `list_id` verbatim.
## Fix Focus Areas
- tinyagentos/projects/lists_store.py[10-24]
- tinyagentos/projects/lists_store.py[176-182]
## Suggested fix
- Make `list_id` a required parameter for `_get_next_position(project_id, list_id)` and remove the `or ""` fallback; or
- If `None` is intended to mean “across all lists in project”, change the SQL to omit the `list_id = ?` predicate when `list_id is None`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
nemotron-ultra-orB review VERDICT: Code has correctness and security issues that need fixing.
Automated first-pass review by the nemotron-ultra-orB lane. The lead still reviews before merge. |
|
nemotron-super review VERDICT: Blocking issue found
Automated first-pass review by the nemotron-super lane. The lead still reviews before merge. |
|
Closing per Jay's decision (dec-gqor2k: close both, re-card clean). Red on BOTH test (3.12) and lint since 2026-07-26, and holding one of six fleet throttle slots - which is what starved PR throughput. Re-carded fresh so a lane redoes it against current dev rather than diagnosing a stale red build. No work is lost: the card carries the same scope. |
Autonomous build of board card tsk-gq43l6.
Files:
tinyagentos/app.py | 7 ++
tinyagentos/projects/init.py | 16 +++
tinyagentos/projects/lists_store.py | 182 ++++++++++++++++++++++++++++++
tinyagentos/routes/agent_auth_requests.py | 13 ++-
4 files changed, 217 insertions(+), 1 deletion(-)
Summary by CodeRabbit
lists_readandlists_writeaccess scopes for external agents.