Merge dev into master: v1.0.0-beta.43#2087
Merged
Merged
Conversation
The invite list serializers passed the raw JSON-encoded TEXT column straight through, so clients received scopes as a string. The Projects invite dialog calls scopes.join() expecting an array, which threw and tripped the SPA error boundary the moment any pending invite existed. The same crash fired on the post-mint list refresh, closing the dialog before the URL and PIN were shown (#2065). Parse defensively via _scopes_as_list so a malformed row degrades to [] instead of crashing every consumer. Fixes the crash path of #2065.
…ew round (#2069) Pitfalls: runtime state and key material never enter commits (16), new views wire desktop AND mobile registries (17), retrofit columns on shipped stores need guarded ALTERs because the migration runner baselines pre-existing DBs without executing (18), scope deltas vs the issue are stated explicitly (19). Extended pitfall 1 with the house project auth-guard pattern and pitfall 15 with the answer-in-thread requirement. Skill: fold-first now requires in-thread replies per numbered item and bot review freshness checks; new scope-honesty subsection covers deferral notes, follow-up issues, and the git status runtime-artifact check.
) The 15 minute TTL killed invites before a human could hand the link and PIN to an agent operator: both of the maintainer's live-test invites expired unredeemed. Default is now 1 hour, and mint accepts an optional ttl_secs (60s to 24h) so a longer-lived invite is a deliberate choice at mint time rather than a code change. The PIN remains the security gate; expiry is hygiene. Tests: default lands at 1 hour, ttl_secs is honoured, over-cap is 422.
…#2071) * fix(invites): allow revoking expired invites, 409 for terminal states store.revoke only matched status='pending', so an invite that lazily flipped to expired could never be revoked: the route reported 404 while the row kept showing in the pending list. Revoke now covers pending and expired; the route returns 409 with the actual state for redeemed or already-revoked invites instead of a misleading 404. Tests: revoke-expired succeeds (store + route), revoke-redeemed refused, double-revoke 409. * fix(invites): name the mid-redeem state on revoke, cover 409 paths Folds both Kilo suggestions: a 'claimed' invite is mid-redeem, so it now returns a 409 that says so instead of the generic retry text, and both 409 route paths (redeemed and claimed) have direct tests rather than only store-level coverage.
…slot compatibility note (#2075)
…ollections handoff, app UI (#2062) * feat(library): P1 core — LibraryStore, ingest pipeline, processors, collections handoff, app UI Implements the Library app Phase 1 from docs/design/library-app.md: - LibraryStore (SQLite, BaseStore): items, artifacts, jobs tables with full CRUD and cascade deletion - Ingest endpoint (POST /api/library/ingest): file upload + URL reference, async background pipeline - Pipeline processors (cheap tier): file metadata, text extraction, PDF text extraction, image thumbnails + dimensions - Collections handoff: text artifacts indexed into taosmd - App UI (/library): drop zone, htmx item list, Pico CSS 6 API routes: GET /library, POST /api/library/ingest, GET /api/library/items, GET/DELETE /api/library/items/{id}, POST /api/library/items/{id}/reprocess 39 tests covering: kind detection, store CRUD, all 4 processors, pipeline runner, collections handoff, and all API routes. Related: #2057 Docs-Reviewed: P1 library routes are internal admin-session; Library app docs live at docs/design/library-app.md. No external API surface change yet. * fix(library): address CodeRabbit findings — missing-file error, upload cap, task GC, HTMX responses - pipeline: mark missing source files as 'error' instead of silently 'ready' - ingest: stream file uploads in 1MB chunks with 100MB cap, reject oversized - background tasks: retain strong references via module-level _background_tasks set - HTMX: return HTML fragments (.item-card) when HX-Request header is present * fix(library): address Kilo/CodeRabbit review findings - XSS (CRITICAL): escape title with html.escape in _render_item_card - handed_off no longer increments on ImportError (taosmd unavailable) - Move `import taosmd` to module level, skip loop when unavailable - Reuse already-read bytes for taosmd ingestion (avoid double read) - Log info for URL-only items (stored as references, not fetched) * fix(library): folds 2-7 — drop XSS template, wire real collections contract, fix url items, idempotent reprocess, auth tests, provenance Fold 2: Rewrite handoff_to_collections to use qmd HTTP API - POST /collections -> POST /collections/{id}/index contract - No fabricated per-item agent identities - No self-invented manifest.json - qmd_base_url passed from app.state.qmd_client; graceful skip when absent Fold 3: Verified at 102ceae — ImportError no longer counts handed_off Fold 4: URL-only items get reference artifact instead of empty-ready - Reprocess now deletes old artifacts first (idempotent per spec §2) Fold 5: Negative auth tests (401 on all 6 endpoints), taosmd-unreachable test - Existing test_handoff rewritten: asserts count==0 when qmd missing Fold 6: DROP templates/library.html + _is_htmx/_render_*/HTMLResponse - /library route removed; Library UI belongs to the React desktop app - XSS vector deleted in one move Fold 7: data/library/ and data/collections/ added to .gitignore Also: provenance fields (source_url, processed_at, processor) on artifacts test_library_page now asserts 404 (page deleted) test_handoff_with_qmd mocks httpx for real-contract coverage 41/41 library tests pass * fix(library): address Kilo findings — idempotent collection creation, normalise qmd base URL - Look up existing collection_id from item metadata before creating a new one (prevents duplicate collections on reprocess) - Strip trailing slash from qmd_base_url to avoid double-slash in API paths - Persist collection_id in item.meta_json after first creation * fix(library): address Kilo/CodeRabbit findings — index idempotency, verify-GET logging, concurrent reprocess guard - Pass stable 'key' to POST /collections/{id}/index so qmd can upsert rather than append on reprocess (idempotent indexing) - Log warning when verify-GET fails instead of silently swallowing (avoids falling through to duplicate collection creation) - Reject POST /items/{id}/reprocess with 409 when item is already pending or processing (prevents concurrent pipeline races) - Add test_reprocess_while_processing_returns_409 (42/42 pass) * refactor(library): rewire collections handoff to live taosmd 0.4.0 API Replace invented qmd API shapes with the live taosmd 0.4.0 collections contract: - POST /collections: {name, kind: mixed, source_path} → resp['collection']['id'] - POST /collections/{id}/index: no body, 202 async → poll until ready|error - POST /collections/{id}/link: {type: taos, id: item_id} - Bearer auth from SecretsStore (taosmd-admin-token) - Config key memory_url (taosmd :7900) not qmd.url (:7832) - File perms: 0o640 files, 0o750 dirs - Stats-driven return (files_indexed) not per-artifact loop Updated caller in routes/library.py to fetch taosmd URL from config and admin token from SecretsStore. Updated test mocks for real response shapes. PR #2062, FOLD 1. * fix(library): assert taosmd request sequence in handoff test Per CodeRabbit: verify the full create → index → poll → link sequence with call_count and URL assertions, not just the returned count. * fix(library): fix poll nesting bug, TOCTOU CAS, log handoff return, chmod setgid - Unwrap poll_data[collection] same as line 202 for create response (GET /collections/{id} returns nested shape in taosmd 0.4.0) - Use 0o2750 (setgid) chmod for directories and files - Log handoff return value instead of discarding it - Log ImportError at warning, not debug - Record collection_retryable marker on ALL handoff failure modes - TOCTOU CAS: atomic UPDATE with rowcount check for reprocess guard - Replace bare except OSError:pass with logged warnings in delete_item - Remove unused LibraryStore import in library_collections.py - Add issue link #2063 for URL-fetch deferral - Fix test mocks to nested taosmd 0.4.0 shape - Add POST body assertions (name/kind/source_path) in handoff test - Assert artifact counts in test_reprocess_idempotent, not just 202s * fix(library): address jaylfc review — data-loss guard, retryable, vacuum test, stale meta, setgid, CAS rollback, verify-GET Findings 1-8 from PR #2062 follow-up review: 1. DATA-LOSS: Guard reprocess unlink loop to skip storage_path artifacts. Stop recording source file as artifact path in File/Pdf/ImageProcessors. 2. VACUOUS TEST: Fix test_reprocess_idempotent to assert exact count match and item status is ready (was abs() check that passed with zero artifacts). 3. Stale meta_json: Move item_meta parse before httpx try block; use item_meta in exception handlers instead of re-reading stale item dict. 4. retryable marker: Set collection_retryable on ALL failure paths (create >=400, no id, index !=202, index exception, poll status>=400, poll error, poll exception, poll timeout). Clear on successful handoff. 5. Issue citation: Replace #2063 (merged PR) with TODO comment. 6. setgid: Fix file perms from 0o2640 (meaningless on Linux) to 0o640. Keep 0o2750 on directory. 7. CAS rollback: Wrap deletion + scheduling in try/except; revert pending to ready on failure so item isn't stuck forever. 8. verify-GET: Distinguish 404 (fall through to create) from transient failures (set retryable, return 0 — no duplicate collection). * chore(library): add issue #2078 reference, document collection_retryable as scaffolding --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
… cycle (#2079) * docs(skill+pitfalls): three defect classes from the Library P1 review cycle Pitfalls: tolerances do not test invariants (20), shell snippets in template literals must escape the interpolation sigil (21), conflict resolution reverts fixes when done mechanically (22). All three are drawn from real defects in this cycle, including a data-loss bug that a tolerance-based assertion kept green and 225 syntax errors from one unescaped construct. Skill: a verify-before-you-claim section covering tolerance assertions, mocks built from the caller's assumptions rather than the service contract, and compiling before opening a PR. * docs(skill+pitfalls): external-contract mocks need a captured source Strengthens the mock guidance from a general principle into an enforceable rule. Scopes the danger to mocks of services we do not control, requires a captured real response over a hand-composed fixture, asks for one feature detecting integration test per contract, and rejects a follow-up issue as sufficient mitigation since it detaches the caveat from the code. * docs(skill+pitfalls): split contract mocks by whether the owner is reachable Sibling services are not third parties. taOSmd's maintainer is on the A2A bus, so a contract question costs one message. Every mock failure in the 2062 cycle was a guess at something that was free to ask, and asking produced documented envelopes, a self-corrected stats key list, and a capabilities endpoint. Guessing is only justified when there is genuinely nobody to ask. * docs(skill): how an external contributor reaches another team's agent Adds the routing rule that makes pitfall 23 actionable for contributors who are not on the A2A bus: a contract question goes to the commons repo (or taosmd directly, which is public), the lead relays to the owning agent, and the contributor escalates on the PR if it stalls. Marked explicitly as temporary scaffolding that retires when contributors can hold a taOS identity.
Library P1 (ingest pipeline, cheap-tier processors, collections handoff), three invite fixes from live testing (the dialog crash, unrevokable expired invites, and a 15 minute TTL that killed invites before they could be used), the Hailo AI HAT+2 hardware note, and seven new contributor defect classes.
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? |
|
👋 Thanks for the PR! This one targets See CONTRIBUTING.md for the branch model. |
📝 WalkthroughWalkthroughAdds a Library app with SQLite persistence, asynchronous processing, taosmd collection handoff, and protected API routes. Extends invite TTL and revocation handling, normalizes invite scopes, updates release metadata, and expands contributor and hardware documentation. ChangesLibrary foundation
Invite lifecycle
Release guidance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant LibraryRoutes
participant Pipeline
participant LibraryStore
participant taosmd
Client->>LibraryRoutes: Upload file or submit URL
LibraryRoutes->>LibraryStore: Create pending item
LibraryRoutes->>Pipeline: Schedule background processing
Pipeline->>LibraryStore: Store artifacts and ready status
Pipeline->>taosmd: Create or reuse collection
taosmd-->>Pipeline: Collection ready with indexed count
Pipeline->>taosmd: Link collection to library item
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@CHANGELOG.md`:
- Around line 40-44: Update the contributor documentation changelog entry to say
“eight” instead of “seven,” leaving the listed defect classes unchanged.
In `@docs/contributor-pitfalls.md`:
- Around line 212-214: Update the external-response fixture guidance in
docs/contributor-pitfalls.md lines 212-214 and
.claude/skills/taos-development-skill/SKILL.md lines 312-315 to prohibit
committing raw responses and require minimal fixtures captured from safe test
accounts with secrets, tokens, cookies, PII, and restricted fields removed. Add
this sanitization requirement before the instruction to commit the service
response as a fixture in both sites.
In `@docs/getting-started.md`:
- Around line 34-35: The AI HAT+2 description currently misidentifies the shared
resource as the Raspberry Pi 5’s M.2 slot. Update docs/getting-started.md lines
34-35 and CHANGELOG.md lines 37-39 to state that the AI HAT+2 uses the Pi 5’s
PCIe interface, which is also used by the M.2 HAT+ for NVMe, while preserving
the recommendation to use USB storage alongside the accelerator.
In `@tests/projects/test_invite_store.py`:
- Around line 342-353: Update both tests/projects/test_invite_store.py:342-353
and tests/test_routes_project_invites.py:658-668 to call mark_redeemed() after
redeem() and before the existing assertions, ensuring they exercise the redeemed
state rather than only the claimed state.
In `@tinyagentos/library_pipeline.py`:
- Around line 260-265: Declare pypdf as a runtime dependency in pyproject.toml
so PDF extraction is available on fresh installs. In
tinyagentos/library_pipeline.py lines 209-211, preserve the existing PDF
import/extraction behavior while ensuring the optional ImportError no longer
occurs; the ImageProcessor class at lines 260-265 requires no direct change.
In `@tinyagentos/projects/invite_store.py`:
- Around line 184-186: Update the TTL validation in mint() to reject values
below 60 seconds or above 86400 seconds, while continuing to allow None to use
_EXPIRY_SECS. Perform this validation before calculating expires_ts or
persisting the invite, and retain the existing ValueError behavior for invalid
TTLs.
In `@tinyagentos/routes/library.py`:
- Around line 230-241: Update the list_items endpoint to validate and clamp
limit and offset before passing them to store.list_items: enforce non-negative
offset, enforce a positive bounded limit, and cap oversized values at the
endpoint’s maximum page size. Keep the existing kind/status filters and response
shape unchanged.
- Around line 64-75: Update create_app and _get_library_store so library_store
is initialized during application setup and assigned to app.state.library_store
before requests are served. Remove the lazy unsynchronized check-and-set path in
_get_library_store, while preserving retrieval of the initialized store for
route handlers.
🪄 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: 9b227e47-1c5b-416c-897e-f0ff18e1a204
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.claude/skills/taos-development-skill/SKILL.md.gitignoreCHANGELOG.mddesktop/package.jsondocs/contributor-pitfalls.mddocs/getting-started.mdpyproject.tomltests/projects/test_invite_store.pytests/test_library.pytests/test_routes_project_invites.pytinyagentos/__init__.pytinyagentos/library_collections.pytinyagentos/library_pipeline.pytinyagentos/library_store.pytinyagentos/projects/invite_store.pytinyagentos/routes/__init__.pytinyagentos/routes/library.pytinyagentos/routes/project_invites.py
Comment on lines
+40
to
+44
| - Contributor docs gained seven new defect classes drawn from real review | ||
| findings, covering runtime state in commits, mobile view registries, retrofit | ||
| migrations on shipped stores, scope honesty, tolerance assertions, shell | ||
| snippets in template literals, conflict resolution, and how an external | ||
| contributor reaches another team's agent (#2069, #2079). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the defect-class count.
This says “seven” but lists eight classes: runtime state, mobile registries, migrations, scope honesty, tolerance assertions, shell snippets, conflict resolution, and external-agent coordination.
🤖 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 `@CHANGELOG.md` around lines 40 - 44, Update the contributor documentation
changelog entry to say “eight” instead of “seven,” leaving the listed defect
classes unchanged.
Comment on lines
+212
to
+214
| **Genuinely third-party (GitHub, OpenRouter, Reddit): capture, do not compose.** | ||
| There is no one to ask, so call the real service once and commit its response as | ||
| the fixture. A recorded response cannot encode a wrong belief. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require sanitized fixtures for captured external responses.
Both rules should prohibit raw response commits and require minimal fixtures from safe test accounts with secrets, tokens, cookies, PII, and restricted fields removed.
docs/contributor-pitfalls.md#L212-L214: add the sanitization requirement before “commit its response as the fixture.”.claude/skills/taos-development-skill/SKILL.md#L312-L315: apply the same requirement to the external-service testing workflow.
📍 Affects 2 files
docs/contributor-pitfalls.md#L212-L214(this comment).claude/skills/taos-development-skill/SKILL.md#L312-L315
🤖 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 `@docs/contributor-pitfalls.md` around lines 212 - 214, Update the
external-response fixture guidance in docs/contributor-pitfalls.md lines 212-214
and .claude/skills/taos-development-skill/SKILL.md lines 312-315 to prohibit
committing raw responses and require minimal fixtures captured from safe test
accounts with secrets, tokens, cookies, PII, and restricted fields removed. Add
this sanitization requirement before the instruction to commit the service
response as a fixture in both sites.
Comment on lines
+34
to
+35
| > **AI HAT+2 (Hailo-10H) and the Pi 5 M.2 slot:** The AI HAT+2 NPU accelerator occupies the Raspberry Pi 5's only M.2 slot, the same slot normally used for an NVMe boot drive. You cannot use the HAT and an NVMe SSD at the same time. If you want fast storage alongside Hailo LLM acceleration, use a USB SSD instead. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use PCIe terminology consistently for AI HAT+2.
The AI HAT+2 connects through Raspberry Pi 5’s PCIe port; the M.2 HAT+ provides the M.2 connector for NVMe and uses that same PCIe interface. Update both descriptions while preserving the valid recommendation to use USB storage alongside the accelerator. (raspberrypi.com)
docs/getting-started.md#L34-L35: replace “occupies the Raspberry Pi 5’s only M.2 slot” with the PCIe-sharing explanation.CHANGELOG.md#L37-L39: use the same corrected hardware terminology in the release notes.
📍 Affects 2 files
docs/getting-started.md#L34-L35(this comment)CHANGELOG.md#L37-L39
🤖 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 `@docs/getting-started.md` around lines 34 - 35, The AI HAT+2 description
currently misidentifies the shared resource as the Raspberry Pi 5’s M.2 slot.
Update docs/getting-started.md lines 34-35 and CHANGELOG.md lines 37-39 to state
that the AI HAT+2 uses the Pi 5’s PCIe interface, which is also used by the M.2
HAT+ for NVMe, while preserving the recommendation to use USB storage alongside
the accelerator.
Source: MCP tools
Comment on lines
+342
to
+353
| async def test_revoke_redeemed_invite_returns_false(store): | ||
| result = await store.mint( | ||
| project_id="prj-1", | ||
| scopes=[], | ||
| approval_mode="auto", | ||
| check_interval_secs=1800, | ||
| created_by="u", | ||
| ) | ||
| iid = result["record"]["invite_id"] | ||
| pin = result["pin"] | ||
| await store.redeem(iid, pin) | ||
| assert await store.revoke(iid) is False |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise the redeemed state, not only claimed.
redeem() leaves the invite claimed; both tests therefore miss the actual redeemed-state behavior.
tests/projects/test_invite_store.py#L342-L353: callmark_redeemed()afterredeem()before assertingrevoke()returnsFalse.tests/test_routes_project_invites.py#L658-L668: callmark_redeemed()afterredeem()so the route’sstatus == "redeemed"branch is covered.
📍 Affects 2 files
tests/projects/test_invite_store.py#L342-L353(this comment)tests/test_routes_project_invites.py#L658-L668
🤖 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/projects/test_invite_store.py` around lines 342 - 353, Update both
tests/projects/test_invite_store.py:342-353 and
tests/test_routes_project_invites.py:658-668 to call mark_redeemed() after
redeem() and before the existing assertions, ensuring they exercise the redeemed
state rather than only the claimed state.
Comment on lines
+260
to
+265
| class ImageProcessor(Processor): | ||
| """Image processor — records dimensions, creates thumbnail. | ||
|
|
||
| Thumbnail generation requires Pillow (PIL), which is always available in | ||
| the taOS dev dependencies. | ||
| """ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether pypdf and Pillow are declared as runtime (not dev-only) dependencies
fd -t f 'pyproject.toml|requirements*.txt|setup.cfg|setup.py' --exec sh -c \
'echo "== $1 =="; rg -niP "pillow|pypdf|pypdf2" "$1" || echo " (no match)"' _ {}Repository: jaylfc/taOS
Length of output: 340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dependency declarations =="
sed -n '1,120p' pyproject.toml
echo
echo "== pypdf / Pillow references in repo =="
rg -n -i 'pypdf|pillow|pypdf2' .
echo
echo "== library_pipeline.py relevant sections =="
sed -n '180,290p' tinyagentos/library_pipeline.pyRepository: jaylfc/taOS
Length of output: 34439
Declare pypdf as a runtime dependency tinyagentos/library_pipeline.py#L209-L211 swallows ImportError, so PDF text extraction silently no-ops on fresh installs unless pypdf is added to pyproject.toml.
📍 Affects 1 file
tinyagentos/library_pipeline.py#L260-L265(this comment)tinyagentos/library_pipeline.py#L209-L211
🤖 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/library_pipeline.py` around lines 260 - 265, Declare pypdf as a
runtime dependency in pyproject.toml so PDF extraction is available on fresh
installs. In tinyagentos/library_pipeline.py lines 209-211, preserve the
existing PDF import/extraction behavior while ensuring the optional ImportError
no longer occurs; the ImageProcessor class at lines 260-265 requires no direct
change.
Comment on lines
+184
to
+186
| if ttl_secs is not None and ttl_secs <= 0: | ||
| raise ValueError("ttl_secs must be positive") | ||
| expires_ts = now + (ttl_secs if ttl_secs is not None else _EXPIRY_SECS) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Enforce the TTL bounds in the store as well.
mint() accepts any positive value, bypassing the route’s 60–86400 second contract for direct callers. Validate both bounds here before persisting the expiry.
Proposed fix
- if ttl_secs is not None and ttl_secs <= 0:
- raise ValueError("ttl_secs must be positive")
+ if ttl_secs is not None and not 60 <= ttl_secs <= 86_400:
+ raise ValueError("ttl_secs must be between 60 and 86400")📝 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.
Suggested change
| if ttl_secs is not None and ttl_secs <= 0: | |
| raise ValueError("ttl_secs must be positive") | |
| expires_ts = now + (ttl_secs if ttl_secs is not None else _EXPIRY_SECS) | |
| if ttl_secs is not None and not 60 <= ttl_secs <= 86_400: | |
| raise ValueError("ttl_secs must be between 60 and 86400") | |
| expires_ts = now + (ttl_secs if ttl_secs is not None else _EXPIRY_SECS) |
🤖 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/invite_store.py` around lines 184 - 186, Update the TTL
validation in mint() to reject values below 60 seconds or above 86400 seconds,
while continuing to allow None to use _EXPIRY_SECS. Perform this validation
before calculating expires_ts or persisting the invite, and retain the existing
ValueError behavior for invalid TTLs.
Comment on lines
+64
to
+75
| async def _get_library_store(request: Request): | ||
| """Get the LibraryStore from app.state (lazily initialised).""" | ||
| store = getattr(request.app.state, "library_store", None) | ||
| if store is None: | ||
| from tinyagentos.library_store import LibraryStore | ||
|
|
||
| data_dir = getattr(request.app.state, "data_dir", None) | ||
| base = Path(data_dir) if data_dir else Path(__file__).parent.parent.parent / "data" | ||
| store = LibraryStore(base / "library.db") | ||
| await store.init() | ||
| request.app.state.library_store = store | ||
| return store |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether library_store is created at app startup (vs. only lazily here)
rg -nP 'library_store' -g '!tests/**' -C2Repository: jaylfc/taOS
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files mentioning create_app or app.state ==\n'
rg -n "create_app|app\.state|library_store|LibraryStore" tinyagentos -g '!tests/**' || true
printf '\n== Top-level file tree around app/routes ==\n'
git ls-files tinyagentos | sed -n '1,200p'
printf '\n== library.py outline ==\n'
ast-grep outline tinyagentos/routes/library.py --view expanded || trueRepository: jaylfc/taOS
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app.py around library_store startup ==\n'
sed -n '1540,1675p' tinyagentos/app.py | cat -n
printf '\n== app.py around shutdown cleanup ==\n'
sed -n '1468,1495p' tinyagentos/app.py | cat -n
printf '\n== routes/library.py around helper ==\n'
sed -n '1,90p' tinyagentos/routes/library.py | cat -nRepository: jaylfc/taOS
Length of output: 12817
Initialize library_store before serving requests — _get_library_store still does an unsynchronized check-then-set, and create_app doesn’t assign app.state.library_store, so concurrent first hits can build duplicate SQLite-backed stores. tinyagentos/routes/library.py:64-75
🤖 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/library.py` around lines 64 - 75, Update create_app and
_get_library_store so library_store is initialized during application setup and
assigned to app.state.library_store before requests are served. Remove the lazy
unsynchronized check-and-set path in _get_library_store, while preserving
retrieval of the initialized store for route handlers.
Comment on lines
+230
to
+241
| @router.get("/api/library/items") | ||
| async def list_items( | ||
| request: Request, | ||
| kind: str | None = None, | ||
| status: str | None = None, | ||
| limit: int = 50, | ||
| offset: int = 0, | ||
| ): | ||
| """List library items, optionally filtered by kind or status.""" | ||
| store = await _get_library_store(request) | ||
| items = await store.list_items(kind=kind, status=status, limit=limit, offset=offset) | ||
| return {"items": items, "count": len(items)} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp limit/offset before querying.
limit and offset are forwarded unvalidated to the SQL LIMIT ? OFFSET ?. A negative limit maps to LIMIT -1 in SQLite, which returns all rows (unbounded), and there is no upper bound to prevent oversized page requests.
🛡️ Suggested clamp
- store = await _get_library_store(request)
- items = await store.list_items(kind=kind, status=status, limit=limit, offset=offset)
+ store = await _get_library_store(request)
+ limit = max(1, min(limit, 200))
+ offset = max(0, offset)
+ items = await store.list_items(kind=kind, status=status, limit=limit, offset=offset)
return {"items": items, "count": len(items)}📝 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.
Suggested change
| @router.get("/api/library/items") | |
| async def list_items( | |
| request: Request, | |
| kind: str | None = None, | |
| status: str | None = None, | |
| limit: int = 50, | |
| offset: int = 0, | |
| ): | |
| """List library items, optionally filtered by kind or status.""" | |
| store = await _get_library_store(request) | |
| items = await store.list_items(kind=kind, status=status, limit=limit, offset=offset) | |
| return {"items": items, "count": len(items)} | |
| `@router.get`("/api/library/items") | |
| async def list_items( | |
| request: Request, | |
| kind: str | None = None, | |
| status: str | None = None, | |
| limit: int = 50, | |
| offset: int = 0, | |
| ): | |
| """List library items, optionally filtered by kind or status.""" | |
| store = await _get_library_store(request) | |
| limit = max(1, min(limit, 200)) | |
| offset = max(0, offset) | |
| items = await store.list_items(kind=kind, status=status, limit=limit, offset=offset) | |
| return {"items": items, "count": len(items)} |
🤖 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/library.py` around lines 230 - 241, Update the list_items
endpoint to validate and clamp limit and offset before passing them to
store.list_items: enforce non-negative offset, enforce a positive bounded limit,
and cap oversized values at the endpoint’s maximum page size. Keep the existing
kind/status filters and response shape unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotion of dev to master for v1.0.0-beta.43. Clean merge, no conflicts.
Contents
Library app P1 (#2062): LibraryStore, ingest pipeline, cheap-tier processors (file, text, PDF, image) and the collections handoff to taOSmd over the live Collections API, with async index polling and typed link rows. Three review rounds, including a reprocess bug that deleted the user's original uploaded file and a test whose tolerance was keeping it green.
Three invite fixes, all surfaced by live use rather than tests:
ttl_secs(feat(invites): 1 hour default TTL + optional per-mint ttl_secs #2072)Docs: Hailo-10H AI HAT+2 hardware entry with the Pi 5 M.2 slot conflict (#2075), and seven new contributor defect classes from this cycle's reviews (#2069, #2079).
Verification
1.0.0b43inuv.lock;test_version_lock_syncpasses.Summary by CodeRabbit
New Features
Bug Fixes
Documentation