Conversation
…ken env is read (#2277)
…otifications (#2280) * feat(notifications): admin-gated POST /api/notifications to create notifications Add an ADMIN-gated POST /api/notifications accepting {title, message, level=info, source, data} that calls NotificationStore.add() so SSE and PWA web-push fire for review-request notifications. Previously notifications were only creatable via the internal store, forcing orchestrators and lead agents to do raw DB inserts that skip SSE and web-push. Includes a test asserting non-admin sessions get 403 and that an admin-created notification appears in GET /api/notifications. * docs(changelog): admin notification-create endpoint
…6) (#2275) * tsk-3lkyq2 [OPEN] taOStalk s1: ToolCallBlock + StatusBlock/question * docs(changelog): tool_call and status content-block renderers
* taOStalk s1: show live thinking badge on bound session channels - add taostalk_agent to Channel settings type - compute thinkingChannelIds from channels whose taostalk_agent matches a currently thinking agent in the existing typingAgents state - pass thinkingChannelIds to ChannelSidebar - render a pulsing amber dot next to thinking session channels in both mobile and desktop sidebar rows - tests: badge appears on thinking start, absent otherwise, clears on end * a11y(chat): give the thinking badge an accessible name, plus changelog The badge was aria-hidden with a title attribute, so the one piece of information it conveys - that an agent is working - was unavailable to screen readers, and title is not exposed when aria-hidden removes the element from the accessibility tree. role=img with an aria-label names it without turning every channel row into a live region.
* tsk-6ylbbx [OPEN] taOStalk s1: TextBlock + ThinkingBlock renderers ( * docs(changelog): text and thinking content-block renderers * fix(chat): use the shell surface token for the thinking toggle hover MessagesApp.palette.test.ts forbids hardcoded white/black utilities in this file; the new disclosure button used hover:bg-white/[0.06]. Swapped for hover:bg-shell-surface-hover, the token the rest of the file already uses, rather than adding a palette-ok waiver - the waiver would silence the check instead of satisfying it.
* Add a NOTES area to the Projects app - a place for the user (and agent * docs: project notes area, project_notes scope, and its agent API surface
…etch wrappers (#2289) * fix(desktop): add CSRF protection to mutating requests in library, framework-api, memory-api, x-monitor Four lib modules were using raw fetch() on mutating HTTP methods (POST/PUT/DELETE) without the withCsrf wrapper, bypassing the double-submit CSRF check. Affected functions: - library.ts: deleteLibraryItem, reprocessLibraryItem, ingestLibraryUrl - framework-api.ts: startFrameworkUpdate, setPermittedModels - memory-api.ts: setMemoryModel - x-monitor.ts: postJson/putJson helpers, deleteWatch Fixes #2126 * fix(desktop): install auth guard in chat and standalone app PWAs chat-main.tsx and app-standalone-main.tsx were missing installAuthGuard(), leaving the chat PWA and standalone app PWA without CSRF protection on window.fetch calls. Only main.tsx (desktop shell PWA) had the guard. Add the import + call to both entry modules, matching the pattern in main.tsx. installAuthGuard is idempotent (module-level flag), so double install is safe if an entry point is loaded more than once in the same realm. Add entry-guards.test.ts asserting all three entry modules install the auth guard on import. * fix(desktop): normalise Headers instance in x-monitor fetchJson, add regression test withCsrf() returns a Headers instance at csrf.ts:33. Spreading that instance into an object literal (the old fetchJson line 56 pattern) silently drops every entry because Headers entries are not own- enumerable properties. The CSRF token the PR was written to add was therefore never reaching the wire. Fix by building a fresh Headers with the Accept default and copying caller headers via forEach — the iterator visits all entries correctly. Add a regression test that exercises createWatch → postJson → fetchJson → withCsrf with a stubbed csrf_token cookie, asserting the token survives into the final fetch call. * fix(desktop): preserve Headers instance in fetchJson wrappers Do not spread init.headers into a plain object literal — this drops multi-value header support and breaks Headers-method access. When withCsrf() returns a live Headers instance, the old pattern silently discarded the X-CSRF-Token header because Headers objects have no enumerable own properties. Instead construct a fresh Headers from init?.headers (handles both plain-object and Headers inputs), set Accept, and pass the Headers directly to fetch(). Also fixes fetchJsonOrThrow in memory.ts and standardises x-monitor.ts to match the same pattern. Closes CodeRabbit finding on PR #2165. * fix(test): update header assertions for Headers instance (not plain object) The Headers-spread fix in the wrapper modules now passes native Headers objects to fetch() instead of plain {Accept: ...} objects. Tests that accessed opts.headers.Accept fail because Headers.get() is required. Updated all 24 assertions across 7 test files. * fix(test): handle both Headers and plain-object in Accept assertions Some fetch wrappers (fetchJson) now pass Headers instances, but direct fetch() calls still use plain objects. Use opts.headers?.get?.("Accept") ?? opts.headers?.Accept to handle both cases. * fix(test): use dual-mode header assertions for Content-Type in github/knowledge/memory The CSRF fix converted fetch wrappers to pass Headers instances (see 2273290). a7cf0af updated the Accept assertions to the dual-mode pattern (opts.headers?.get?.() ?? opts.headers?.[...]) but left the Content-Type assertions reading a plain-object index, so the six postJson/putJson tests received undefined. Apply the same dual-mode pattern to Content-Type so Headers-instance and plain-object callers both resolve. --------- Co-authored-by: Hogne <227774406+hognek@users.noreply.github.com>
Every agent runs the same scripts from ~/.taos-team/, so a pattern kill is a fleet-wide kill. It fired in both directions in twelve hours on 2026-08-04: three watcher deaths, two wrong diagnoses, each agent having checked whether a process looked like theirs rather than whose it was. Records the owner check (env, not cmdline) and the three ways of writing it that silently do not work. Docs-Reviewed: this IS the coordination doc; the rule it adds is the doc change.
* tsk-fgj5yp [OPEN] Rename Tasks app to Routines (#2143) * fix(docs): rename Tasks in the manual SOURCE, not the generated file The PR edited docs/taos-agent-manual.md, which is generated from docs/agent-manual/*.md by scripts/build-agent-manual.py. The source still said Tasks, so test_compiled_output_matches_committed failed and the next person to run the build would have silently reverted the rename. Fixed the source and regenerated. Changelog added as a fragment.
….http (#2299) projectsApi.http built its default headers with a plain-object spread (...(init?.headers || {})). When init.headers was a Headers instance (as returned by withCsrf downstream), the spread silently dropped every entry, and the trailing ...init spread overwrote the merged headers key entirely, clobbering Content-Type. Mirror the x-monitor/library fix: build a fresh Headers via new Headers(init?.headers), set Content-Type on it, and spread ...init before headers so merged headers win.
…2304) Add desktop/src/App.tsx, desktop/src/components/**, and desktop/src/stores/** to the when_changed glob for the user-visible-changelog rule so modifications to the desktop shell require a changelog entry, matching the existing coverage for desktop/src/apps/*/**.
An unparseable or missing doc-gate config previously raised an unhandled traceback that exited 1 -- identical to a real documentation-drift violation, so a typo in docs/doc-gate.toml was indistinguishable from a missing changelog. Catch tomllib.TOMLDecodeError and OSError in main(), print a clear error to stderr, and exit 2 instead.
…2283) * docs: add taos-agent OS skill for OS-native agent operation Add .claude/skills/taos-agent/SKILL.md consolidating the agent-manual OS-operation content into actionable instructions for the OS-native taOS agent. Features the hard rule that all desktop driving goes only through POST /api/desktop/command + POST /api/desktop/screenshot. Covers opening and driving apps and windows, desktop control, projects, files, memory and notes, chat conventions, image generation, and answering the user. Updates docs/agent-manual/index.md to point at the new skill and at the existing taos-development-skill. Adds a CHANGELOG entry. Draft for @taOS-dev review. * docs(skill): state that desktop control needs a session, not a registry JWT * ci: re-trigger checks on the current head doc-gate's failure was recorded at 11:16Z against the PREVIOUS head; the rebase at 41dce53 landed after it and doc-gate never re-reported, leaving a stale required check blocking the PR. Verified clean locally against current dev. * docs: reword 'desktop/window control' so the invariants scan stops reading it as a path check_doc_gate.py's Layer A scans index.md for referenced repo paths and treated the slash in 'desktop/window control' as a path that does not exist. It is prose, not a path. 'desktop and window control' reads better anyway.
…ard) _do_approve never passed defer_binding to approve_request_record, so the flag added in PR 2187 was dead code: the project_id-required 400 guard still fired on the defer combo, and when project_id was present the token was bound anyway despite defer_binding:true. - Pass defer_binding=body.defer_binding at the call site. - Add a 400 guard for defer_binding:true with an explicit (non-blank) project_id -- the two are contradictory. - Route tests covering the defer combo (unbound token, unbound grants, no membership row, no a2a channel), the 400 guard, and the unchanged non-deferred path. Red run on buggy head (before the fix): test_defer_with_project_scopes_no_project_id_succeeds_unbound -> 400 (expected 200) test_defer_with_explicit_project_id_returns_400 -> 200 (expected 400) Lifecycle note on never-bound deferred identities: An unbound deferred identity is minted with project_id=None on both the token and every grant. It is currently indistinguishable from a global identity: no expiry, no pending binding flag, no visibility surface in the Permissions app. A follow-up card should track (a) a lifecycle/expiry mechanism so deferred-but-never-bound grants do not persist forever, and (b) a visibility surface so operators can see and reap stale deferred identities.
…figs Pr #2306 left two config-error paths that still exited 1 instead of the config-error code: - Structurally invalid config (valid TOML, wrong shape, e.g. rules is a string) passed load_config and died later in the rule loop with an AttributeError. Add _validate_config to check the shape after parsing. - Non-UTF-8 config bytes raised UnicodeDecodeError inside tomllib.load (which decodes before parsing), uncaught by the TOMLDecodeError-only handler. Catch that alongside the other config errors. Also move EXIT_CONFIG_ERROR from 2 to 3: exit code 2 is argparse's own usage-error exit, so a typo'd flag was indistinguishable from a broken config. Now 1 (violation), 2 (argparse), and 3 (config error) are all mutually distinct. Supersedes exec/tsk-thh54d (pr #2306).
…ctive-handle 409 Documents the behaviour added in this PR: approving with defer_binding when the agent already holds an active handle returns 409 and points at assign-agent, rather than advising a duplicate identity. Clears the two doc-gate rules the PR was failing (user-visible-changelog and agent-manual).
…dev/spa-deps-0488a9d012 chore(deps): bump the spa-deps group in /desktop with 12 updates
Add token_min_iat INTEGER column (default 0) to agent_registry so tokens
can be invalidated per-identity without revoking the whole identity.
- migration _migration_v6_add_token_min_iat: idempotent ALTER TABLE with
default 0 so the migration cannot lock the fleet out
- AgentRegistryStore.bump_token_min_iat(canonical_id, ts): persist cutoff
- Auth path (_verify_agent_scope): after the status check, reject tokens
whose iat claim is < the record's token_min_iat with 401 'token superseded'
- POST /api/agents/registry/{id}/rotate-tokens: session-owner/admin route
that bumps the cutoff and writes a forensic audit-log entry
- Tests: store-level (bump sets cutoff, default zero, nonexistent nil),
auth-path (old token rejected, new token passes, default-zero safe),
route (admin rotates, owner rotates, nonexistent 404)
- bump_token_min_iat: use MAX(token_min_iat, ?) so the cutoff is monotonically non-decreasing — a stale/lower timestamp cannot silently un-supersede tokens - rotate-tokens route: handle None from bump_token_min_iat (race condition) with 404 instead of AttributeError on updated.get() - audit entry: capture before_token_min_iat and after_token_min_iat in the governance audit payload so rotation values are traceable - tests: remove unused token bindings in route-level tests
Docs-Reviewed: CI workflow changes are rebase artifacts (restoring dev's baseline). Token rotation API is internal to agent_registry, not user-visible. Agent behavior unchanged — this is a registry-side enforcement of token freshness.
tsk-mtds32 [OPEN] S4e: device pair-requests + grant Decision (consen
…shrunk file Review fixes on the lane's version: the _PersistentSessions copy of the two prune methods referenced self._sessions_file, an attribute that class does not have (AttributeError if ever called) - deleted; AuthManager's startup pruner loaded, pruned a throwaway dict and never wrote back, so the grown file only shrank on the next session mint - it now loads (which prunes) and saves once, and the log reports before -> after rather than after -> after. Added the test that fails on the decorative version: the sessions FILE must shrink at init. Docs-Reviewed: internal cleanup of an unmerged branch; behaviour is the carded fix itself
Auth sessions never pruned: 37,077 entries, 4.8MB and growing
…t, reuse client fixture BLOCKER #2: test_owner_can_rotate was a copy of test_admin_can_rotate — both used the same admin session. Replaced with three real tests: - test_nonadmin_owner_can_rotate_own_identity (→200) - test_nonadmin_cannot_rotate_others_agent (→403) - test_empty_userid_agent_is_admin_only (→403) BLOCKER #3: Added TestMigrationV5ExistingDB which builds the pre-change agent_registry schema by hand (no token_min_iat column), inserts a row, inits the store, and asserts the row gained token_min_iat=0 with old tokens still authenticating. SHOULD-FIX #4: agent_app fixture now depends on the shared client fixture and only inits agent_registry + agent_grants on top, removing ~200 lines of copy-paste that would break when new stores land in conftest.
Fix-forward PR 2306 (doc-gate config errors): 2 config-error paths still exit 1
Fix-forward PR 2308 (consent defer): untested defer+active-handle path, 409 advises a duplicate identity
registry: per-identity token rotation via token_min_iat (fixes #2205)
Collate 5 changelog.d fragments into the 1.0.0-beta.47 section (merged with the direct [Unreleased] entries under single headings), bump the four version files including uv.lock's normalized 1.0.0b47. Docs-Reviewed: release version bump; no CI, packaging or contribution rule change
chore(release): v1.0.0-beta.47
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
👋 Thanks for the PR! This one targets See CONTRIBUTING.md for the branch model. |
📝 WalkthroughWalkthroughThe release adds backend stores and routes for project notes, device pairing, device blocking, notifications, token rotation, and deferred binding. It also adds desktop chat and library features, request-security updates, documentation, changelog tooling, tests, and beta.47 metadata. ChangesBackend platform capabilities
Desktop experience updates
Documentation and release tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Device as Pairing device
participant API as device_pair_requests routes
participant Store as DevicePairRequestsStore
participant Decisions as Decisions route
participant Devices as DeviceStore
Device->>API: POST /api/device-pair-requests
API->>Store: create pending request
API->>Decisions: create device_pairing decision
Device->>API: GET request status
Decisions->>Store: set accepted or denied
Decisions->>Devices: register approved device
Decisions->>Store: claim scoped token once
Device->>API: poll accepted request
Possibly related PRs
🚥 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 |
![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
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (18)
desktop/src/apps/codingstudio/BuildView.test.tsx-4-4 (1)
4-4: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
vi.mockedfor the mocked import.
streamTaosAgentChatis still typed as the production function, so callingstreamTaosAgentChat.mockImplementation(...)relies onany. Create a typed mock reference and callmockImplementationon it.Proposed fix
import { streamTaosAgentChat } from "../appstudio/stream-chat"; +const mockStreamTaosAgentChat = vi.mocked(streamTaosAgentChat); + ... - streamTaosAgentChat.mockImplementation( + mockStreamTaosAgentChat.mockImplementation( ... - streamTaosAgentChat.mockImplementation( + mockStreamTaosAgentChat.mockImplementation(Also applies to: 51-52, 83-84
🤖 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/apps/codingstudio/BuildView.test.tsx` at line 4, Update the BuildView.test.tsx tests to create a typed mock reference for streamTaosAgentChat using vi.mocked, and call mockImplementation through that reference instead of the imported production function. Apply the same change to the additional mocked import usages at the referenced test sections.docs/agent-coordination.md-69-69 (1)
69-69: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a fenced code block for the
/proccommand.
markdownlint-cli2reports MD046 at Line 69 because the command uses an indented code block. Replace it with a fencedshblock.🤖 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/agent-coordination.md` at line 69, Replace the indented code block containing the /proc environment and kill command in the agent coordination documentation with a fenced sh code block, preserving the command unchanged.Source: Linters/SAST tools
desktop/src/apps/LibraryApp.tsx-147-152 (1)
147-152: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the persisted settings shape and storage cap.
Valid JSON is not necessarily valid
LibrarySettings. A negative cap produces a negative progress value. A source-rule field that is an object causes a React render error in the settings table.parseIntalso truncates fractional input.Validate quality, action, source-rule fields, and a finite non-negative integer cap before storing or rendering values. Fall back to defaults for invalid persisted fields.
Also applies to: 1229-1233
🤖 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/apps/LibraryApp.tsx` around lines 147 - 152, Validate each parsed LibrarySettings field before returning it from the persisted-settings loader, including preferred quality, action, and source-rule entries. Accept storage_cap_bytes only when it is a finite, non-negative integer; reject fractional or negative values. Ensure source_rules and their fields have the expected shapes before rendering, and fall back to the corresponding DEFAULT_LIBRARY_SETTINGS values for every invalid field.desktop/src/apps/chat/ChannelSidebar.tsx-64-65 (1)
64-65: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPropagate thinking state to project channel rows.
thinkingChannelIdsonly reaches the direct section rows. The desktop project loop andProjectsSectionMobiledo not receive this prop. Active project channels therefore do not show the thinking indicator in standalone Messages.Pass the IDs into both project-channel renderers and render the same indicator for matching channel IDs.
🤖 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/apps/chat/ChannelSidebar.tsx` around lines 64 - 65, Propagate thinkingChannelIds from ChannelSidebar into the desktop project-channel loop and ProjectsSectionMobile, then render the existing thinking indicator for project rows whose channel ID is included. Preserve the current direct-section behavior and reuse the established indicator logic and prop names in the project row renderers.desktop/src/apps/chat/__tests__/render-helpers.test.tsx-102-116 (1)
102-116: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a valid
ToolCallContentBlockfixture.Line 107 omits required
call_idandstatus. The catch-allContentBlockmember accepts the malformed object, so this test rendersToolCallBlockwith no status indicator.Proposed fix
- { kind: "tool_call", name: "grep", args: {} }, + { kind: "tool_call", call_id: "c1", name: "grep", status: "running" },🤖 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/apps/chat/__tests__/render-helpers.test.tsx` around lines 102 - 116, Update the tool_call fixture in the renderContent test to include valid ToolCallContentBlock fields, specifically call_id and status, so ToolCallBlock renders through its normal status-aware path. Keep the existing assertions and other content fixtures unchanged.docs/doc-gate.toml-66-66 (1)
66-66: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the duplicate
[[rules]]declaration.Line 66 creates an empty rule table before
user-visible-changelog. Remove one declaration so the configuration contains only the intended rule.🤖 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/doc-gate.toml` at line 66, Remove the duplicate empty [[rules]] declaration adjacent to user-visible-changelog in doc-gate.toml, leaving only the intended rule table.scripts/collate_changelog.py-45-56 (1)
45-56: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject non-bullet fragment content.
parse_fragmentaccepts titles and ordinary prose as changelog entries. This conflicts with the documented malformed-fragment failure behavior and can publish invalid release notes.Require a
-bullet for each entry. Allow an indented continuation only after a bullet.🤖 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 `@scripts/collate_changelog.py` around lines 45 - 56, Update parse_fragment to accept entries only when lines begin with “- ”, rejecting titles or ordinary prose as malformed fragment content. Track whether a bullet is currently active, append indented continuation lines only after that bullet, and raise the existing no-bullets error when no valid bullets are found..claude/skills/taos-agent/SKILL.md-59-62 (1)
59-62: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle the screenshot 409 response explicitly.
The guidance describes a missing desktop as
delivered == 0, butPOST /api/desktop/screenshotreturns HTTP 409 when no desktop is connected. State the separate handling for command delivery, screenshot/layout 409 responses, and 504 timeouts.Suggested wording
- If no desktop is connected (`delivered` is 0) or the desktop did not respond in time (504), report that to the user rather than retrying into a void. + For `/api/desktop/command`, report `delivered == 0` as no connected desktop. + For screenshot and layout reads, report HTTP 409 as no connected desktop and HTTP 504 as a timeout.🤖 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 @.claude/skills/taos-agent/SKILL.md around lines 59 - 62, Update the desktop-communication guidance near the delivered/504 handling to explicitly distinguish command delivery failures from screenshot or layout requests: treat delivered == 0 as no connected desktop, report HTTP 409 from screenshot/layout endpoints as the corresponding unavailable-desktop response, and report HTTP 504 as a timeout; do not retry or use alternate automation paths..claude/skills/taos-agent/SKILL.md-113-113 (1)
113-113: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the current display name
Routines.The skill still lists
Tasks, whiledesktop/src/registry/app-registry.ts:63and both manuals useRoutines. Keep thetasksidentifier unchanged, but update the user-facing name here.Suggested fix
- Other bundled apps (Library, Channels, Secrets, Tasks, MCP, Guides and more); if you do not know one, guess from its name and point the user to Guides. + Other bundled apps (Library, Channels, Secrets, Routines, MCP, Guides and more); if you do not know one, guess from its name and point the user to Guides.🤖 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 @.claude/skills/taos-agent/SKILL.md at line 113, Update the user-facing bundled-app list in the skill text to use “Routines” instead of “Tasks,” while preserving the underlying tasks identifier unchanged..claude/skills/taos-agent/SKILL.md-342-345 (1)
342-345: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winQualify the privacy claims.
State that taOS core storage and built-in update telemetry stay local as described, but data sent through Browser/terminal traffic, cloud providers, or apps can leave the host. Remove or verify the statement about IP retention.
🤖 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 @.claude/skills/taos-agent/SKILL.md around lines 342 - 345, Update the “Is my data private?” section to qualify its claims: state that taOS core storage and built-in update telemetry remain local, while Browser/terminal traffic, configured cloud providers, and apps may send data outside the host. Remove the unconditional “only two things ever leave” wording and remove or verify any claim about IP retention.desktop/package.json-90-90 (1)
90-90: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRaise the Node.js floor to match Vite 8.
desktop/package.jsondeclares Vite 8, whoseengines.nodeis^20.19.0 || >=22.12.0; CI usesactions/setup-nodewithnode-version: "20", but that does not lock the floor to Vite 8’s minimum. Addengines.nodeor an.nvmrc/package manager runtime to 20.19+ so contributors and packaging environments cannot install under an incompatible Node.js.🤖 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/package.json` at line 90, Update the desktop package runtime requirements alongside the Vite dependency declaration to enforce Node.js ^20.19.0 or >=22.12.0. Prefer adding the package’s engines.node field so contributors, CI, and packaging environments reject incompatible Node versions.Source: MCP tools
tinyagentos/routes/agent_registry.py-789-798 (1)
789-798: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAudit the persisted cutoff value.
bump_token_min_iatusesMAX(token_min_iat, ts). If an existing cutoff is later thants,after_token_min_iat=tsrecords a value that was not stored. Useupdated["token_min_iat"]in the audit payload.🤖 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_registry.py` around lines 789 - 798, Update the _audit_governance call in the rotate-tokens flow so after_token_min_iat uses the persisted cutoff from updated["token_min_iat"] rather than the requested ts value, preserving the actual MAX-based result in the audit record.tinyagentos/device_pair_requests_store.py-1-19 (1)
1-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winModule docstrings are placed after
from __future__ import annotationsin both new pairing files. A string literal is only the module docstring when it is the first statement in the file. In both files the future import runs first, so the text becomes a discarded expression and__doc__isNone. Tools such ashelp(),pydoc, and IDE hovers show no documentation for either module.
tinyagentos/device_pair_requests_store.py#L1-L19: move the store docstring abovefrom __future__ import annotations.tinyagentos/routes/device_pair_requests.py#L1-L28: move the route docstring abovefrom __future__ import annotations.🤖 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/device_pair_requests_store.py` around lines 1 - 19, Move the module docstring before the future import in tinyagentos/device_pair_requests_store.py lines 1-19 and tinyagentos/routes/device_pair_requests.py lines 1-28. Ensure each docstring is the first statement so the corresponding module’s __doc__ and documentation tools expose it.tinyagentos/auth.py-60-67 (1)
60-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLegacy float session entries are never pruned.
_prune_expiredskips any entry that is not adict. Legacy entries are plain floats holdingexpires_at(see_get_session_entryat line 671 andcleanup_sessionsat line 735). Expired legacy entries therefore stay in the file forever, which defeats the startup shrink for installs that predate the dict shape.🔧 Proposed fix
def _prune_expired(self, data: dict) -> None: now = time.time() - expired_keys = [ - token for token, entry in data.items() - if isinstance(entry, dict) and entry.get("expires_at") is not None and now >= entry.get("expires_at") - ] + expired_keys = [] + for token, entry in data.items(): + if isinstance(entry, (int, float)): + exp = float(entry) + elif isinstance(entry, dict): + exp = entry.get("expires_at") + else: + continue + if exp is not None and now >= exp: + expired_keys.append(token) for token in expired_keys: del data[token]🤖 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/auth.py` around lines 60 - 67, Update _prune_expired to recognize legacy float session entries as expiration timestamps in addition to dictionary entries, and remove them when now is at or past their expiry. Preserve the existing handling for dictionary entries and leave unrecognized entry shapes untouched.tinyagentos/routes/project_notes.py-23-30 (1)
23-30: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd length limits to
CreateNoteInandUpdateNoteIn.
titleandbodyaccept any string length. An approved agent holding aproject_notesgrant can write arbitrarily large rows intoproject_notes, which growsprojects.dbwithout bound. Addmax_lengthconstraints so the route rejects oversized payloads with a 422.🛡️ Proposed fix
-from pydantic import BaseModel +from pydantic import BaseModel, Field @@ class CreateNoteIn(BaseModel): - title: str - body: str = "" + title: str = Field(max_length=200) + body: str = Field(default="", max_length=100_000) class UpdateNoteIn(BaseModel): - title: str | None = None - body: str | None = None + title: str | None = Field(default=None, max_length=200) + body: str | None = Field(default=None, max_length=100_000)🤖 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/project_notes.py` around lines 23 - 30, Add max_length constraints to the title and body fields in CreateNoteIn and UpdateNoteIn so oversized payloads are rejected with a 422, while preserving their existing required, default, and optional behavior.tinyagentos/routes/decisions.py-529-529 (1)
529-529: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDo not mint a device against the
"user"sentinel.If
decision["user_id"]is empty,decided_bybecomes the literal string"user", and line 552 registers a device owned by a user id that matches no account. That device never appears in any owner's device list, so the owner cannot revoke or block it. Creation guards against a missing admin today, so this path should be unreachable. Fail closed rather than relying on that.🛡️ Proposed fix
- decided_by = decision.get("user_id") or "user" + decided_by = decision.get("user_id") + if not decided_by: + logger.warning( + "device pairing: decision %s has no user_id; refusing to mint", + decision.get("id"), + ) + return True🤖 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/decisions.py` at line 529, Update the decision handling around decided_by so an empty decision["user_id"] fails closed instead of falling back to the literal "user" sentinel. Ensure device creation cannot proceed without a valid user ID, while preserving normal behavior for decisions with a present user_id and the existing registration flow.tests/test_project_notes_store.py-47-53 (1)
47-53: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNewest-first ordering tests can flake on identical
created_atvalues.ProjectNotesStore.list_notessorts bycreated_at DESCwith no secondary key, andcreate_notestampscreated_atwithtime.time(). Notes created back-to-back in a test can receive the same float timestamp, and SQLite then returns them in an arbitrary order.
tests/test_project_notes_store.py#L47-L53: three notes are created with no delay, then a strict[c, b, a]order is asserted. Either add a tiebreaker tolist_notes(for exampleORDER BY created_at DESC, id DESC) and keep the assertion, or force distinct timestamps between thecreate_notecalls.tests/test_routes_project_notes.py#L111-L121: the same pattern with two notes and a strict[b, a]assertion. Apply the same fix so both sites depend on a deterministic ordering contract.A tiebreaker in
list_notesis the better fix, because it makes the ordering deterministic for production callers as well, not only for tests.🤖 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_project_notes_store.py` around lines 47 - 53, The newest-first ordering is nondeterministic when notes share created_at timestamps. Update ProjectNotesStore.list_notes to order by created_at descending with id descending as the secondary key; this root-cause fix applies to tests/test_project_notes_store.py lines 47-53 and tests/test_routes_project_notes.py lines 111-121, which require no direct changes.tests/test_contacts_peer.py-762-790 (1)
762-790: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPin the real signing key, otherwise the tamper assertion is vacuous.
The contact is registered with
ed25519_pub="pk", which is not the key that signed the envelope. Signature verification therefore fails for the untampered envelope as well. The test passes even if tamper detection is removed, so it proves only that a wrong pinned key returns 403.
test_inbox_nonce_replayat Line 666 shows the correct setup: pin the local identity signing pubkey so a valid envelope succeeds. Then the tampered envelope isolates the tamper path.🐛 Proposed fix
async def test_inbox_tampered_signature_rejected(self, client_with_contacts, app_with_contacts): """A tampered envelope must be rejected with 403 at the route level.""" local_id = resolve_local_identity_id() + # Pin the key that actually signs envelopes, so only the tampering + # can be the cause of the rejection. + from tinyagentos.hub.identity import public_identity as _hub_pub + signing_pub = _hub_pub()["signing_pubkey"] + store = app_with_contacts.state.contacts_store await store.add_contact( contact_id="hub:tamper-test", hub_username="tamper-test", display_name="T", - ed25519_pub="pk", x25519_pub="ek", + ed25519_pub=signing_pub, x25519_pub="ek", )🤖 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_contacts_peer.py` around lines 762 - 790, Update test_inbox_tampered_signature_rejected to register the contact with the actual local identity Ed25519 signing public key, matching the setup used by test_inbox_nonce_replay, instead of the placeholder "pk". Keep the envelope tampering unchanged so the untampered signature is valid and the 403 assertion specifically verifies tamper detection.
🧹 Nitpick comments (14)
desktop/src/stores/__tests__/reduce-effects.test.ts (1)
31-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the production
data-perfbehavior.This test manually sets
data-perfbefore asserting it. It passes if theuseEffectindesktop/src/App.tsxis removed or inverted.Render the production behavior, or extract the root-attribute update into a shared helper and test that helper.
🤖 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/stores/__tests__/reduce-effects.test.ts` around lines 31 - 45, Update the reduce-effects test around useThemeStore and the document root so it exercises the production data-perf synchronization rather than manually setting or removing the attribute. Render the relevant App behavior, or extract its root-attribute update logic into a shared helper and test that helper for both enabled and disabled reduceEffects states.tinyagentos/routes/device_pair_requests.py (1)
175-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the swallowed notification failure.
The Decision path at line 160 logs its failure, but this handler discards the exception. A broken notification store is then invisible. Log at warning level for parity.
🔧 Proposed fix
- except Exception: - pass + except Exception: + logger.warning( + "device_pair_requests: bell notification failed", exc_info=True + )🤖 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/device_pair_requests.py` around lines 175 - 176, Update the exception handler in the device-pair notification flow to log the caught notification-store failure at warning level before continuing, matching the logging behavior of the Decision path while preserving the existing exception-swallowing behavior.Source: Linters/SAST tools
tinyagentos/routes/project_notes.py (1)
127-135: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
update_notecan returnnullwith status 200.
store.update_notereturnsNonewhen the row no longer exists. A concurrent delete between line 124 and line 127 makes this handler return a JSONnullbody with status 200. Return the existence-hiding 404 instead, so the client sees a consistent contract.🔧 Proposed fix
updated = await store.update_note( note_id, title=payload.title, body=payload.body, ) + if updated is None: + return JSONResponse({"error": "not found"}, status_code=404)🤖 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/project_notes.py` around lines 127 - 135, In the note update handler, validate the result of store.update_note before logging activity or returning it. When updated is None, return the existing existence-hiding 404 response; otherwise preserve the current activity logging and successful return behavior.tinyagentos/auth.py (2)
52-58: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winWrite the sessions file atomically.
_save_prunedtruncates and rewrites the session file in place. If the process stops during the write, every live session is lost or the file becomes invalid JSON. Startup pruning now rewrites the whole file on every boot, so the exposure grows. Write to a temporary file in the same directory and thenos.replaceit.♻️ Proposed refactor
def _save_pruned(self, data: dict) -> None: self._prune_expired(data) - self._path.write_text(json.dumps(data)) + tmp = self._path.with_suffix(self._path.suffix + ".tmp") + tmp.write_text(json.dumps(data)) + os.replace(tmp, self._path)🤖 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/auth.py` around lines 52 - 58, Update Auth’s _save_pruned method to serialize the pruned session data to a temporary file in the same directory, then atomically replace self._path with os.replace. Ensure the temporary file is safely cleaned up on failure while preserving the existing _prune_expired behavior.
197-222: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTake the session lock during startup pruning.
_prune_sessions_on_startupcallsself._sessions._load()andself._sessions._save(data)outsideself._sessions._lock. Every other read-modify-write path in_PersistentSessionsholds that lock. The call currently runs in__init__before requests arrive, so this is defensive only, but a secondAuthManageron the same data directory would race. Consider adding a locked helper on_PersistentSessionsinstead of reaching into the private methods.🤖 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/auth.py` around lines 197 - 222, Update _prune_sessions_on_startup to perform the load-and-save pruning operation under _PersistentSessions._lock, preferably by adding a dedicated locked helper on _PersistentSessions and calling that helper instead of accessing _load() and _save() directly. Preserve the existing removed-count calculation and logging behavior.tinyagentos/device_pair_requests_store.py (1)
72-84: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
_is_expiredfails open on an unparsable timestamp.If
expires_at_tscannot be parsed, the function returnsFalse, so the request stays live forever. ReturnTruefor a corrupt value so a damaged row expires instead of remaining approvable.🛡️ Proposed fix
try: exp = datetime.fromisoformat(expires_at_ts) except ValueError: - return False + # A corrupt timestamp must not leave the request pending forever. + return True🤖 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/device_pair_requests_store.py` around lines 72 - 84, Update _is_expired so a non-empty expires_at_ts that raises ValueError in datetime.fromisoformat is treated as expired by returning True; preserve the existing False behavior for missing expiry values and normal timestamp comparisons.tinyagentos/projects/notes_store.py (1)
56-63: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider pagination for
list_notes.
list_notesreturns every note for a project in one response. Notes accumulate over a project's life. Addlimitandoffsetparameters with a default cap so the route response stays bounded.♻️ Proposed signature change
- async def list_notes(self, project_id: str) -> list[dict]: + async def list_notes( + self, project_id: str, limit: int = 200, offset: int = 0 + ) -> list[dict]: async with self._db.execute( - "SELECT * FROM project_notes WHERE project_id = ? ORDER BY created_at DESC", - (project_id,), + "SELECT * FROM project_notes WHERE project_id = ? " + "ORDER BY created_at DESC LIMIT ? OFFSET ?", + (project_id, limit, offset), ) as cur:🤖 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/notes_store.py` around lines 56 - 63, Update list_notes to accept limit and offset parameters, applying a sensible default maximum limit and the offset in its SQL query so responses remain bounded while preserving the existing project_id filter and newest-first ordering. Ensure callers can override the limit when needed.tinyagentos/routes/decisions.py (1)
565-569: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueA failure inside the applier leaves the pair request
pending.If
device_store.registerorset_decisionraises, the Decision is already answered but the pair request keepsstatus = 'pending'. No new Decision is raised for it. The polling device only recovers when the TTL passes and_live_statusreportsexpired, which is up to_TTL_SECS. Consider transitioning the request todeniedin the exception handler so the device learns the outcome immediately.🤖 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/decisions.py` around lines 565 - 569, Update the exception handler in the device-pairing grant applier to transition the associated pair request to denied when device_store.register or set_decision fails, before logging and returning. Preserve the existing warning and ensure the failure does not leave the request pending.tests/test_contacts_peer.py (1)
792-820: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm the freshness check runs before signature verification.
This test also pins
ed25519_pub="pk", so the signature can never verify. It expects 400 rather than 403, which holds only while the route checks envelope freshness before the signature. If that order changes, the test starts asserting the wrong status for the wrong reason.Pinning the real signing pubkey, as in
test_inbox_nonce_replayat Line 666, would make the 400 depend only on the future timestamp.🤖 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_contacts_peer.py` around lines 792 - 820, Update test_inbox_future_timestamp_rejected to use the valid signing public key setup from test_inbox_nonce_replay instead of the placeholder ed25519_pub="pk", so signature verification can succeed and the expected 400 specifically confirms freshness validation occurs before signature verification.tests/test_auth.py (2)
139-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this test or make it construct a new
AuthManager.The name states "at startup", but the body never constructs a second
AuthManager. It callscleanup_sessions()on the existing instance, which duplicatestest_cleanup_sessionsat Line 129. Startup pruning is exercised by_prune_sessions_on_startup, which only runs inAuthManager.__init__.♻️ Proposed change to exercise startup pruning
- def test_prune_expired_sessions_at_startup(self, tmp_path): - """Expired entries are pruned when AuthManager is initialized.""" - mgr = AuthManager(tmp_path) - # Create an expired session (will be written to file) - token = mgr.create_session(user_id="uid1") - # Manually expire it and force prune - mgr._sessions[token] = {"user_id": "uid1", "expires_at": time.time() - 1, "long_lived": False} - mgr.cleanup_sessions() - assert token not in mgr._sessions - - # Create a valid session - token2 = mgr.create_session(user_id="uid2") - # Verify it's still valid - assert mgr.validate_session(token2) is not None + def test_prune_expired_sessions_at_startup(self, tmp_path): + """Expired entries are pruned when AuthManager is initialized.""" + mgr = AuthManager(tmp_path) + expired = mgr.create_session(user_id="uid1") + live = mgr.create_session(user_id="uid2") + mgr._sessions[expired] = { + "user_id": "uid1", "expires_at": time.time() - 1, "long_lived": False + } + + # A fresh manager over the same data dir must prune at construction. + reloaded = AuthManager(tmp_path) + assert expired not in reloaded._sessions + assert reloaded.validate_session(live) is not None🤖 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_auth.py` around lines 139 - 152, Update test_prune_expired_sessions_at_startup to persist an expired session, then construct a new AuthManager instance so _prune_sessions_on_startup runs during AuthManager.__init__; assert the expired token is removed and a valid session remains usable, without calling cleanup_sessions on the original manager.
209-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the no-op property, or drop this test.
The body is identical to
test_prune_at_startup_preserves_unexpiredat Line 166 except for the token string. It asserts preservation, not that pruning is a no-op. Compare the file content before and after construction to test the stated behavior.♻️ Proposed change
def test_prune_already_clean_is_noop(self, tmp_path): """Pruning an already-clean store (no expired entries) is a no-op.""" - # Create a file with no expired entries (simulating clean state) token1 = "valid_token_789" session_data = {token1: {"user_id": "uid1", "expires_at": time.time() + 3600, "long_lived": False}} sessions_file = tmp_path / ".auth_sessions" sessions_file.write_text(json.dumps(session_data)) - - # Create new manager - should preserve the valid session - mgr2 = AuthManager(tmp_path) - assert token1 in mgr2._sessions + + AuthManager(tmp_path) + + # Nothing was removed: the persisted entry set is unchanged. + assert json.loads(sessions_file.read_text()) == session_data🤖 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_auth.py` around lines 209 - 219, Update test_prune_already_clean_is_noop to capture the sessions file contents before constructing AuthManager, then compare them with the file contents afterward to assert pruning is a true no-op; retain the existing valid-session setup and remove the redundant in-memory membership-only assertion.tests/test_routes_agent_auth_requests.py (1)
1385-1414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated store setup into a fixture, and release stores in
finally.All four tests in
TestDeferBindingApprovalrepeat the same block: constructAgentRegistryStore,AuthRequestsStore,AgentGrantsStore, andProjectStore, init each, load the keypair, then monkeypatch the sameapp.stateattributes.The stores are also closed by plain
awaitcalls at the end of each test. If an assertion fails, those calls never run and the aiosqlite connections stay open for the rest of the session. A fixture with teardown fixes both problems.♻️ Proposed fixture
`@pytest_asyncio.fixture` async def defer_env(client, monkeypatch, tmp_path): """Real stores wired into app.state for the defer_binding approval tests.""" from tinyagentos.agent_registry_store import ( AgentRegistryStore, load_or_create_signing_keypair, ) from tinyagentos.auth_requests_store import AuthRequestsStore from tinyagentos.agent_grants_store import AgentGrantsStore from tinyagentos.projects.project_store import ProjectStore registry = AgentRegistryStore(tmp_path / "reg.db") auth_store = AuthRequestsStore(tmp_path / "auth.db") grants = AgentGrantsStore(tmp_path / "grants.db") pstore = ProjectStore(tmp_path / "projects.db") stores = (registry, auth_store, grants, pstore) for s in stores: await s.init() priv, pub = load_or_create_signing_keypair(tmp_path / "keys") state = client._transport.app.state monkeypatch.setattr(state, "agent_registry", registry) monkeypatch.setattr(state, "auth_requests", auth_store) monkeypatch.setattr(state, "agent_grants", grants) monkeypatch.setattr(state, "project_store", pstore) monkeypatch.setattr(state, "agent_registry_keypair", (priv, pub)) try: yield SimpleNamespace( client=client, registry=registry, auth_store=auth_store, grants=grants, pstore=pstore, pub=pub, ) finally: for s in stores: await s.close()Each test then drops its setup and teardown blocks and uses
defer_env.🤖 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_agent_auth_requests.py` around lines 1385 - 1414, Extract the repeated store initialization, keypair setup, and app.state monkeypatching from all four tests in TestDeferBindingApproval into a shared async defer_env fixture. Have the fixture yield the client, stores, and public key, then close every initialized store in a finally block so cleanup runs after assertion failures. Update each test to use defer_env and remove its duplicated setup and direct teardown calls.tests/test_project_notes_store.py (1)
120-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe test name claims migration over an existing database, but the body creates a fresh one.
ProjectNotesStore(db)at Line 126 runs against an empty path, so no pre-existing schema is migrated. The docstring already concedes this ("on a fresh install"), which contradicts the name. As written, the test cannot fail if a retrofit path for older databases is removed.
tests/test_device_store.pyLines 108-149 shows the pattern that does test a retrofit: build the old table by hand, then init the store.Either rename this test to
test_index_created_on_fresh_db, or build a pre-existingproject_notestable without the index first. Also close the store infinally, because the assert at Line 133 currently runs beforeawait s.close().♻️ Proposed minimal fix
`@pytest.mark.asyncio` -async def test_migration_creates_index_on_existing_db(tmp_path): +async def test_index_created_on_fresh_db(tmp_path): """A notes table on a fresh install carries the created_at DESC index that list_notes relies on for ordering.""" import sqlite3 db = tmp_path / "projects.db" s = ProjectNotesStore(db) await s.init() - conn = sqlite3.connect(str(db)) - idxs = [r[0] for r in conn.execute( - "SELECT name, tbl_name FROM sqlite_master WHERE type='index' AND tbl_name='project_notes'" - )] - conn.close() - assert "idx_notes_project_created" in idxs - await s.close() + try: + conn = sqlite3.connect(str(db)) + try: + idxs = [r[0] for r in conn.execute( + "SELECT name FROM sqlite_master " + "WHERE type='index' AND tbl_name='project_notes'" + )] + finally: + conn.close() + assert "idx_notes_project_created" in idxs + finally: + await s.close()🤖 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_project_notes_store.py` around lines 120 - 134, Correct test_migration_creates_index_on_existing_db so it genuinely covers an existing database: create the legacy project_notes table without the index before ProjectNotesStore.init(), or rename it to test_index_created_on_fresh_db if only fresh-install behavior is intended. Ensure await s.close() runs in a finally block so the store is closed even when the assertion fails.tests/test_routes_project_notes.py (1)
27-41: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClose only the stores this fixture opened.
Setup initializes
agent_registryandagent_grantsonly whenstore._db is None. Teardown closes them unconditionally. If another fixture already opened a store, this fixture closes a store it does not own, and leavesapp.stateholding a closed store for later tests.
tests/test_token_rotation.py(Lines 23-35) handles the same two stores and deliberately does not close them for this reason.Track what setup opened and close only that.
♻️ Proposed fix
`@pytest_asyncio.fixture` async def ctx(client): """Reuse the session-admin `client` app and additionally init the agent registry + grants stores so tokens can be minted against real stores.""" app = client._transport.app + opened = [] for attr in ("agent_registry", "agent_grants"): store = getattr(app.state, attr) if store._db is None: await store.init() + opened.append(store) uid = app.state.auth.find_user("admin")["id"] - yield SimpleNamespace(client=client, app=app, uid=uid) - for attr in ("agent_registry", "agent_grants"): - store = getattr(app.state, attr) - if store._db is not None: - await store.close() + try: + yield SimpleNamespace(client=client, app=app, uid=uid) + finally: + for store in opened: + await store.close()🤖 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_project_notes.py` around lines 27 - 41, Update the ctx fixture’s setup and teardown to track which stores were initialized by this fixture. In the loops over agent_registry and agent_grants, record only stores whose _db was None before calling init, then close only those recorded stores during teardown; leave pre-existing stores open.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a6d731bb-1f1b-4657-80a0-b3e42518934c
⛔ Files ignored due to path filters (2)
desktop/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonuv.lockis excluded by!**/*.lock,!**/uv.lock
📒 Files selected for processing (98)
.claude/skills/taos-agent/SKILL.mdCHANGELOG.mdchangelog.d/.gitkeepdesktop/package.jsondesktop/src/App.tsxdesktop/src/__tests__/entry-guards.test.tsdesktop/src/app-standalone-main.tsxdesktop/src/apps/ActivityApp.test.tsxdesktop/src/apps/LibraryApp.settings.test.tsxdesktop/src/apps/LibraryApp.test.tsxdesktop/src/apps/LibraryApp.tsxdesktop/src/apps/MediaPlayerApp.test.tsxdesktop/src/apps/MessagesApp.tsxdesktop/src/apps/ProjectsApp/ProjectMembers.test.tsxdesktop/src/apps/TasksApp.tsxdesktop/src/apps/chat/ChannelSidebar.tsxdesktop/src/apps/chat/__tests__/ChannelSidebar.test.tsxdesktop/src/apps/chat/__tests__/content-blocks.test.tsxdesktop/src/apps/chat/__tests__/render-helpers.test.tsxdesktop/src/apps/codingstudio/BuildView.test.tsxdesktop/src/apps/codingstudio/CodeView.test.tsxdesktop/src/chat-main.tsxdesktop/src/components/Desktop.tsxdesktop/src/components/StatusBlock.tsxdesktop/src/components/ToolCallBlock.tsxdesktop/src/components/__tests__/ConfirmDialog.test.tsxdesktop/src/components/__tests__/StatusBlock.test.tsxdesktop/src/components/__tests__/ToolCallBlock.test.tsxdesktop/src/lib/agent-browsers.tsdesktop/src/lib/framework-api.tsdesktop/src/lib/github.test.tsdesktop/src/lib/github.tsdesktop/src/lib/knowledge.test.tsdesktop/src/lib/knowledge.tsdesktop/src/lib/library.tsdesktop/src/lib/mail.test.tsdesktop/src/lib/memory-api.test.tsdesktop/src/lib/memory-api.tsdesktop/src/lib/memory.test.tsdesktop/src/lib/memory.tsdesktop/src/lib/models.test.tsdesktop/src/lib/projects.test.tsdesktop/src/lib/projects.tsdesktop/src/lib/reddit.test.tsdesktop/src/lib/reddit.tsdesktop/src/lib/x-monitor.tsdesktop/src/lib/youtube.tsdesktop/src/registry/app-registry.tsdesktop/src/stores/__tests__/reduce-effects.test.tsdesktop/tests/x-monitor.test.tsdocs/agent-coordination.mddocs/agent-manual/04-apps.mddocs/agent-manual/index.mddocs/changelog-fragments.mddocs/design/plan-desktop-shell-core.mddocs/design/remote-desktop-app-audit.mddocs/doc-gate.tomldocs/gitea.mddocs/superpowers/plans/2026-04-12-x-monitor.mddocs/superpowers/specs/2026-04-12-app-runtime-design.mddocs/superpowers/specs/2026-04-12-github-browser-design.mddocs/superpowers/specs/2026-04-12-x-monitor-design.mddocs/taos-agent-manual.mdpyproject.tomlscripts/check_doc_gate.pyscripts/collate_changelog.pytests/conftest.pytests/routes/test_device_pair_requests.pytests/routes/test_devices.pytests/test_agent_registry_store.pytests/test_auth.pytests/test_collate_changelog.pytests/test_contacts_peer.pytests/test_device_store.pytests/test_doc_gate.pytests/test_project_notes_store.pytests/test_routes_agent_auth_requests.pytests/test_routes_notifications.pytests/test_routes_project_notes.pytests/test_token_rotation.pytinyagentos/__init__.pytinyagentos/agent_registry_store.pytinyagentos/agent_token_auth.pytinyagentos/app.pytinyagentos/auth.pytinyagentos/auth_middleware.pytinyagentos/device_pair_requests_store.pytinyagentos/device_store.pytinyagentos/projects/ids.pytinyagentos/projects/notes_store.pytinyagentos/routes/__init__.pytinyagentos/routes/agent_auth_requests.pytinyagentos/routes/agent_registry.pytinyagentos/routes/decisions.pytinyagentos/routes/device_pair_requests.pytinyagentos/routes/devices.pytinyagentos/routes/notifications.pytinyagentos/routes/project_notes.py
| Install command (quote exactly): | ||
| `curl -fsSL https://raw.githubusercontent.com/jaylfc/tinyagentos/master/scripts/install-server.sh | sudo bash` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file excerpt =="
if [ -f .claude/skills/taos-agent/SKILL.md ]; then
nl -ba .claude/skills/taos-agent/SKILL.md | sed -n '70,105p'
else
echo "missing .claude/skills/taos-agent/SKILL.md"
fi
echo
echo "== install-server.sh usages in repo =="
rg -n "install-server\.sh|tinyagentos|master|curl.*sha256|sha256sum|gpg|checksum|tag" .claude README.md 2>/dev/null || true
echo
echo "== git status/diff stat =="
git status --short 2>/dev/null || true
git diff --stat 2>/dev/null || trueRepository: jaylfc/taOS
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Python excerpt for target file =="
python3 - <<'PY'
from pathlib import Path
p = Path(".claude/skills/taos-agent/SKILL.md")
if not p.exists():
print("missing .claude/skills/taos-agent/SKILL.md")
else:
lines = p.read_text(encoding="utf-8").splitlines()
for i in range(max(0, len(lines[:90])-10), min(len(lines), 100)):
print(f"{i+1:4}: {lines[i]}")
PY
echo
echo "== install-server.sh usages in repo =="
rg -n "install-server\.sh|tinyagentos|master|curl.*sha256|sha256sum|gpg|checksum|tag|wget" .claude README.md 2>/dev/null || true
echo
echo "== git status/diff stat =="
git status --short 2>/dev/null || true
git diff --stat 2>/dev/null || trueRepository: jaylfc/taOS
Length of output: 14577
Pin the controller installer to a release commit and add verification before running it as root.
This installs from jaylfc/tinyagentos master and pipes directly to sudo bash; an attack on that branch or a branch rename can execute arbitrary root code. Use an immutable release artifact or a release commit SHA, include a checksum/sig verification step, avoid the install pipeline for beta releases, and document how root is used.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 89: [PE2] Sudo/Root Execution: Commands invoke sudo or root privileges. Verify this elevated access is necessary and justified.
Remediation: Avoid sudo/root unless strictly required. Prefer least-privilege patterns. If elevation is needed, document the justification and scope.
(Privilege Escalation (PE2))
[error] 89: [TM2] Chaining Abuse: Tool calls are chained to bypass individual safety checks or escalate capabilities beyond what any single tool call would allow.
Remediation: Limit tool chaining depth and validate the output of each tool before passing it to the next. Require explicit user approval for multi-step chains.
(Tool Misuse (TM2))
🤖 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 @.claude/skills/taos-agent/SKILL.md around lines 88 - 89, Update the
controller installation instructions around the quoted curl command to use an
immutable release artifact or pinned release commit instead of master, verify
its checksum or signature before execution, and avoid piping the downloaded
script directly to sudo bash. For beta releases, provide a non-pipeline
download-and-verify flow, and document the specific point and purpose of root
privileges.
Source: Linters/SAST tools
| const headers = new Headers(init?.headers); | ||
| headers.set("Accept", "application/json"); | ||
| const res = await fetch(url, { ...init, headers }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restore CSRF wrapping in the JSON request helpers.
The Headers rewrite sends a plain RequestInit directly to fetch. The known mutating callers therefore omit X-CSRF-Token. Apply withCsrf to the final same-origin request, or wrap each mutating caller.
- desktop/src/lib/knowledge.ts#L64-L66: wrap the final init;
postJsoncurrently creates an unwrapped POST request. - desktop/src/lib/github.ts#L82-L84: wrap the final init; the POST caller currently passes unwrapped options.
Proposed fix
- const res = await fetch(url, { ...init, headers });
+ const res = await fetch(url, withCsrf({ ...init, headers }));Add the withCsrf import where it is absent. Restrict this approach to same-origin API URLs.
📍 Affects 2 files
desktop/src/lib/knowledge.ts#L64-L66(this comment)desktop/src/lib/github.ts#L82-L84
🤖 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/lib/knowledge.ts` around lines 64 - 66, Restore CSRF wrapping for
the final same-origin JSON request in postJson at
desktop/src/lib/knowledge.ts:64-66 by applying withCsrf to the merged init
before fetch, adding its import if needed. Apply the same final-init wrapping in
the POST caller at desktop/src/lib/github.ts:82-84, ensuring both mutating
requests include X-CSRF-Token without changing handling for non-mutating or
external URLs.
| def _validate_config(config: dict) -> None: | ||
| """Validate the structural shape of a parsed doc-gate config. | ||
|
|
||
| A config that parses as valid TOML but has the wrong shape (e.g. | ||
| ``rules = "not a list"``) is a config error, not a runtime crash: surface | ||
| it as EXIT_CONFIG_ERROR rather than letting it die in the rule loop with | ||
| an AttributeError.""" | ||
| if not isinstance(config, dict): | ||
| raise ValueError("config root must be a table") | ||
| rules = config.get("rules", []) | ||
| if not isinstance(rules, list): | ||
| raise ValueError("'rules' must be a list of tables") | ||
| for i, rule in enumerate(rules): | ||
| if not isinstance(rule, dict): | ||
| raise ValueError(f"rules[{i}] must be a table") | ||
| gate = config.get("gate", {}) | ||
| if not isinstance(gate, dict): | ||
| raise ValueError("'gate' must be a table") | ||
| if "trailer" in gate and not isinstance(gate["trailer"], str): | ||
| raise ValueError("'gate.trailer' must be a string") | ||
| invariants = config.get("invariants", {}) | ||
| if not isinstance(invariants, dict): | ||
| raise ValueError("'invariants' must be a table") | ||
| scan = invariants.get("referenced_paths_scan", []) | ||
| if not isinstance(scan, list): | ||
| raise ValueError("'invariants.referenced_paths_scan' must be a list") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate rule and invariant member types.
_validate_config accepts a rule such as when_changed = 1 because it only checks that the rule is a table. That invalid value then reaches rule evaluation instead of returning EXIT_CONFIG_ERROR.
Validate name, when_changed, require_doc, hint, and on_modify. Also require string entries in referenced_paths_scan.
🤖 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 `@scripts/check_doc_gate.py` around lines 96 - 121, Extend _validate_config to
validate each rule member: require name, when_changed, require_doc, hint, and
on_modify to be strings when present, rejecting invalid values with ValueError
so callers return EXIT_CONFIG_ERROR. Also iterate over
invariants.referenced_paths_scan and require every entry to be a string,
preserving existing structural checks and defaults.
| text = CHANGELOG.read_text(encoding="utf-8") | ||
| if UNRELEASED not in text: | ||
| print(f"collate-changelog: {CHANGELOG.name} has no '{UNRELEASED}' anchor", file=sys.stderr) | ||
| return 1 | ||
| # Insert directly BELOW [Unreleased] so Unreleased stays empty and on top, | ||
| # which is what the release train expects on the next cycle. | ||
| anchor = UNRELEASED + "\n" | ||
| text = text.replace(anchor, anchor + "\n" + section, 1) | ||
| CHANGELOG.write_text(text, encoding="utf-8") | ||
|
|
||
| for path in consumed: | ||
| path.unlink() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make insertion and fragment cleanup safe to retry.
If CHANGELOG.md ends directly after ## [Unreleased], Line 106 passes but Line 112 cannot match the newline-suffixed anchor. The script then deletes every fragment without inserting a release section.
Confirm that exactly one anchor insertion occurred before cleanup. Also detect an existing version section so a retry after interruption between the write and unlink() calls cannot add the release twice.
🤖 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 `@scripts/collate_changelog.py` around lines 105 - 116, Make the insertion
logic in the changelog collation flow robust when [Unreleased] has no trailing
newline, and verify exactly one insertion occurred before deleting consumed
fragments. Before inserting, detect whether the target version section already
exists and skip insertion when it does, so retries cannot duplicate releases;
only perform fragment cleanup after confirming the changelog was successfully
updated or already contains that version section.
| # Reject tokens issued before the identity's token_min_iat cutoff (rotation). | ||
| token_min_iat = record.get("token_min_iat") or 0 | ||
| token_iat = payload.get("iat") or 0 | ||
| if token_iat < token_min_iat: | ||
| raise HTTPException(status_code=401, detail="token superseded") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Apply token rotation to check_agent_identity.
This cutoff check only protects _verify_agent_scope. check_agent_identity validates the same token and active registry record but does not check token_min_iat. A superseded token can still pass identity-gated flows.
Extract the active-token and cutoff validation into a shared helper, then call it from both paths. Add a regression test for a rotated token through check_agent_identity.
🤖 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/agent_token_auth.py` around lines 115 - 119, Extract the
active-token and token_min_iat cutoff validation from _verify_agent_scope into a
shared helper, preserving the existing 401 behavior for superseded tokens.
Invoke this helper from both _verify_agent_scope and check_agent_identity so
rotated tokens are rejected in identity-gated flows, and add a regression test
covering a superseded token passed to check_agent_identity.
| # Project notes: read/write a project's persistent idea notes | ||
| # (title + markdown body) by a session owner/admin or a project-bound | ||
| # project_notes grant holder. Mirrors project_doc_review's single-scope | ||
| # read+write surface for a lightweight persistent notes surface. | ||
| "project_notes", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Bind project_notes through _PROJECT_SCOPES.
project_notes is project-bound, but _PROJECT_SCOPES does not include it. The required explicit project_id check then does not run. A non-deferred approval can use the unauthenticated request record’s project_id as effective_project and issue a grant for a project that the operator did not select.
Add "project_notes" to _PROJECT_SCOPES before release.
Proposed fix
-_PROJECT_SCOPES = {"project_tasks", "project_tasks_create", "project_tasks_update"} | _CANVAS_SCOPES | _FILES_SCOPES
+_PROJECT_SCOPES = {
+ "project_tasks",
+ "project_tasks_create",
+ "project_tasks_update",
+ "project_notes",
+} | _CANVAS_SCOPES | _FILES_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 85 - 89, Add
"project_notes" to the _PROJECT_SCOPES collection so project_notes requests
undergo the required explicit project_id validation and cannot derive
effective_project from an unauthenticated request record when issuing grants.
| ts = int(time.time()) | ||
| before_iat = record.get("token_min_iat") or 0 | ||
| updated = await store.bump_token_min_iat(canonical_id, ts) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Use a rotation value with finer resolution than integer-second iat.
A token minted before rotation in the same Unix second has iat == token_min_iat. The < check accepts it, so rotation does not invalidate every token issued before the request. The current test encodes this ambiguous same-second behavior.
tinyagentos/routes/agent_registry.py#L783-L785: use a signed monotonic token version or a higher-resolution issued-at value for the cutoff.tests/test_token_rotation.py#L166-L174: control the clock and assert that a token issued before rotation in the same second is rejected while a post-rotation token is accepted.
📍 Affects 2 files
tinyagentos/routes/agent_registry.py#L783-L785(this comment)tests/test_token_rotation.py#L166-L174
🤖 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_registry.py` around lines 783 - 785, Update
tinyagentos/routes/agent_registry.py:783-785 in the token-rotation flow around
bump_token_min_iat to use a signed monotonic token version or higher-resolution
issued-at cutoff, rather than integer-second time.time(), so tokens minted
before rotation in the same second are rejected. Update
tests/test_token_rotation.py:166-174 to control the clock and verify that a
pre-rotation same-second token is rejected while a post-rotation token is
accepted.
| admin_id = _admin_user_id(request) | ||
| decision_store = getattr(request.app.state, "decision_store", None) | ||
| if decision_store is not None and admin_id: | ||
| try: | ||
| await decision_store.create( | ||
| from_agent="@taOSc", | ||
| question=( | ||
| f"taOSc on {display!r} wants to connect. Code {verify_code}. Allow?" | ||
| ), | ||
| type="approve_deny", | ||
| priority="blocking", | ||
| project_id=None, | ||
| user_id=admin_id, | ||
| metadata={ | ||
| "kind": "device_pairing", | ||
| "pair_request_id": pair_request_id, | ||
| }, | ||
| context=( | ||
| f"Pairing request from {body.platform} device " | ||
| f"at {requester_ip or 'unknown IP'}" | ||
| ), | ||
| ) | ||
| except Exception: | ||
| logger.warning("device_pair_requests: could not raise Decision", exc_info=True) | ||
|
|
||
| # Best-effort bell notification so the request leaves an auditable | ||
| # trail in the inbox. Must not fail the created request. | ||
| notifs = getattr(request.app.state, "notifications", None) | ||
| if notifs is not None: | ||
| try: | ||
| await notifs.add( | ||
| title="Pair request", | ||
| message=f"{display} wants to connect (code {verify_code})", | ||
| level="warning", | ||
| source="pair_requests", | ||
| user_id=admin_id, | ||
| data={"request_id": pair_request_id, "platform": body.platform}, | ||
| ) | ||
| except Exception: | ||
| pass | ||
|
|
||
| # F3 / criterion 5: verify_code is returned ONLY here -- never on the poll. | ||
| return {"pair_request_id": pair_request_id, "verify_code": verify_code} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A pairing request that cannot raise a Decision still returns 200 and consumes a cap slot.
If decision_store is None or _admin_user_id returns "", the whole approval block is skipped. The route still returns {pair_request_id, verify_code}. The device then polls a request that no human can ever see or approve, and the row holds a pending slot for the full _TTL_SECS window. Repeated calls exhaust _PENDING_CAP for every other device.
Fail the request instead, so the device reports a clear error rather than polling forever.
🔧 Proposed fix
admin_id = _admin_user_id(request)
decision_store = getattr(request.app.state, "decision_store", None)
- if decision_store is not None and admin_id:
+ if decision_store is None or not admin_id:
+ raise HTTPException(
+ status_code=503,
+ detail="pairing is unavailable: no instance admin to approve the request",
+ )
+ if True:
try:Place this check before store.create(...) so no row is written when pairing cannot proceed.
📝 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.
| admin_id = _admin_user_id(request) | |
| decision_store = getattr(request.app.state, "decision_store", None) | |
| if decision_store is not None and admin_id: | |
| try: | |
| await decision_store.create( | |
| from_agent="@taOSc", | |
| question=( | |
| f"taOSc on {display!r} wants to connect. Code {verify_code}. Allow?" | |
| ), | |
| type="approve_deny", | |
| priority="blocking", | |
| project_id=None, | |
| user_id=admin_id, | |
| metadata={ | |
| "kind": "device_pairing", | |
| "pair_request_id": pair_request_id, | |
| }, | |
| context=( | |
| f"Pairing request from {body.platform} device " | |
| f"at {requester_ip or 'unknown IP'}" | |
| ), | |
| ) | |
| except Exception: | |
| logger.warning("device_pair_requests: could not raise Decision", exc_info=True) | |
| # Best-effort bell notification so the request leaves an auditable | |
| # trail in the inbox. Must not fail the created request. | |
| notifs = getattr(request.app.state, "notifications", None) | |
| if notifs is not None: | |
| try: | |
| await notifs.add( | |
| title="Pair request", | |
| message=f"{display} wants to connect (code {verify_code})", | |
| level="warning", | |
| source="pair_requests", | |
| user_id=admin_id, | |
| data={"request_id": pair_request_id, "platform": body.platform}, | |
| ) | |
| except Exception: | |
| pass | |
| # F3 / criterion 5: verify_code is returned ONLY here -- never on the poll. | |
| return {"pair_request_id": pair_request_id, "verify_code": verify_code} | |
| admin_id = _admin_user_id(request) | |
| decision_store = getattr(request.app.state, "decision_store", None) | |
| if decision_store is None or not admin_id: | |
| raise HTTPException( | |
| status_code=503, | |
| detail="pairing is unavailable: no instance admin to approve the request", | |
| ) | |
| if True: | |
| try: | |
| await decision_store.create( | |
| from_agent="`@taOSc`", | |
| question=( | |
| f"taOSc on {display!r} wants to connect. Code {verify_code}. Allow?" | |
| ), | |
| type="approve_deny", | |
| priority="blocking", | |
| project_id=None, | |
| user_id=admin_id, | |
| metadata={ | |
| "kind": "device_pairing", | |
| "pair_request_id": pair_request_id, | |
| }, | |
| context=( | |
| f"Pairing request from {body.platform} device " | |
| f"at {requester_ip or 'unknown IP'}" | |
| ), | |
| ) | |
| except Exception: | |
| logger.warning("device_pair_requests: could not raise Decision", exc_info=True) | |
| # Best-effort bell notification so the request leaves an auditable | |
| # trail in the inbox. Must not fail the created request. | |
| notifs = getattr(request.app.state, "notifications", None) | |
| if notifs is not None: | |
| try: | |
| await notifs.add( | |
| title="Pair request", | |
| message=f"{display} wants to connect (code {verify_code})", | |
| level="warning", | |
| source="pair_requests", | |
| user_id=admin_id, | |
| data={"request_id": pair_request_id, "platform": body.platform}, | |
| ) | |
| except Exception: | |
| pass | |
| # F3 / criterion 5: verify_code is returned ONLY here -- never on the poll. | |
| return {"pair_request_id": pair_request_id, "verify_code": verify_code} |
🧰 Tools
🪛 Ruff (0.16.1)
[error] 175-176: try-except-pass detected, consider logging the exception
(S110)
[warning] 175-175: Do not catch blind exception: Exception
(BLE001)
🤖 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/device_pair_requests.py` around lines 137 - 179, Require
an available decision_store and non-empty admin_id before creating the pairing
request or calling decision_store.create in the device-pairing route. If either
prerequisite is missing, fail the request with a clear client-facing error
instead of returning pair_request_id and verify_code; preserve the existing
best-effort notification and success response only when approval can proceed.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| await feedback_store.close() | ||
| await client_log_store.close() | ||
| await project_element_store.close() | ||
| await project_notes_store.close() |
There was a problem hiding this comment.
CRITICAL: Missing device_pair_requests.close() in test fixture teardown
device_pair_requests is initialized in the client fixture setup (line 433) but never closed in the teardown. This leaks a SQLite connection across every test run that uses the client fixture. Compare the symmetric project_notes_store close that was just added at this same location.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| logger = logging.getLogger(__name__) | ||
|
|
||
| # F5: the store does not validate the platform, so the whitelist lives here. | ||
| _VALID_PLATFORMS = frozenset({"ios", "watchos", "android"}) |
There was a problem hiding this comment.
WARNING: Platform whitelist mismatch with devices route
_VALID_PLATFORMS includes "android", but routes/devices.py RegisterIn.platform_supported only accepts "ios" and "watchos". A pair request for an Android device will succeed, and upon admin approval _apply_device_pairing_grant will mint a device with platform="android" that the normal /api/devices/register route would reject.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| class CreateNotificationRequest(BaseModel): | ||
| title: str | ||
| message: str | ||
| level: str = "info" |
There was a problem hiding this comment.
WARNING: level accepts any string without validation
CreateNotificationRequest.level accepts any string, but the UI HTML renderer at line 40 only knows how to render "info", "warning", and "error". Invalid levels are persisted to the database and silently render with no icon in the notifications list.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| user_id=admin_id, | ||
| data={"request_id": pair_request_id, "platform": body.platform}, | ||
| ) | ||
| except Exception: |
There was a problem hiding this comment.
WARNING: Bare except Exception: pass swallows all errors
The best-effort notification delivery at lines 175-176 catches and discards every exception type, including TypeError, AttributeError, or database failures. This masks real bugs in the auditable notification trail that the comment explicitly says "must not fail the created request". Consider at minimum logging the exception.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| raise RuntimeError("DevicePairRequestsStore not initialised") | ||
| now_iso = _iso(_now()) | ||
| cur = await self._db.execute( | ||
| "SELECT * FROM device_pair_requests " |
There was a problem hiding this comment.
WARNING: list_pending uses SELECT * while get() uses _SAFE_COLS
This inconsistency means any future schema change adding a sensitive column to device_pair_requests would be silently exposed through list_pending but not through get(). Align list_pending with the explicit _SAFE_COLS projection used by get().
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| { kind: "thinking", text: "thinking...", collapsed: true }, | ||
| { kind: "tool_call", call_id: "c1", name: "Bash", status: "running" as const }, | ||
| { kind: "thinking", text: "pondering" }, | ||
| { kind: "tool_call", name: "grep", args: {} }, |
There was a problem hiding this comment.
WARNING: Incomplete tool_call block omits required fields
The test data { kind: "tool_call", name: "grep", args: {} } is missing required fields call_id and status. The runtime cast bypasses type checking, so the block renders with a missing status indicator. The test does not assert on the rendered tool_call output, so it does not verify correct rendering.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
|
|
||
| function StatusIndicator({ status }: { status: ToolCallStatus }) { | ||
| switch (status) { |
There was a problem hiding this comment.
WARNING: StatusIndicator switch has no default case
If status is not "running", "done", or "error" at runtime (backend change or type mismatch), the component returns undefined and React renders nothing. TypeScript exhaustiveness checking should catch this at compile time, but a runtime mismatch would silently render nothing.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const { container } = render(<ToolCallBlock block={blockFor({ status: "error" })} />); | ||
| const card = container.querySelector('[data-tool-call="true"]'); | ||
| expect(card?.getAttribute("data-status")).toBe("error"); | ||
| expect(card?.querySelector(".lucide-triangle-alert")).not.toBeNull(); |
There was a problem hiding this comment.
CRITICAL: Wrong lucide-react class name in test assertion
lucide-react generates the class name lucide-alert-triangle (kebab-case of the icon name alert-triangle), not lucide-triangle-alert. This assertion will fail at runtime.
| expect(card?.querySelector(".lucide-triangle-alert")).not.toBeNull(); | |
| expect(card?.querySelector(".lucide-alert-triangle")).not.toBeNull(); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| // without it, cookie-authenticated project create/update/delete 403 with | ||
| // "CSRF token missing". Non-mutating GETs pass through unchanged. | ||
| const headers = new Headers(init?.headers); | ||
| headers.set("Content-Type", "application/json"); |
There was a problem hiding this comment.
SUGGESTION: headers.set("Content-Type", "application/json") unconditionally overwrites caller-provided headers
The original code spread ...(init?.headers || {}) into the headers object, allowing caller headers to override the default Content-Type. The new code always overwrites any Content-Type in init.headers. While correct for JSON callers, it changes behavior for any caller that intentionally sets a different Content-Type (e.g., multipart/form-data).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| - **Notes** (`notes`), **Todo** (`todo`): shared notes and lists you belong to. | ||
| - **Images** (`images`): generate and manage artwork. | ||
| - **Browser** (`browser`), **Terminal** (`terminal`): web browsing and shell access. | ||
| - Other bundled apps (Library, Channels, Secrets, Tasks, MCP, Guides and more); if you do not know one, guess from its name and point the user to Guides. |
There was a problem hiding this comment.
WARNING: Skill references Tasks instead of Routines
This PR renames the Tasks app to Routines across all other documentation and the app itself, but this skill file still says Tasks. An OS-native agent reading this skill would try to open a non-existent Tasks app instead of Routines.
Reply with @kilocode-bot fix it to to have Kilo Code address this issue.
Code Review SummaryStatus: 15 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (11 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 199.6K · Output: 29.7K · Cached: 2.3M |
|
Merging directly rather than via gate_merge.sh: the gate requires a |
2 participants
Promotes dev to master for the v1.0.0-beta.47 release. See CHANGELOG's 1.0.0-beta.47 section for contents (10 merges today: device pairing consent loop, device block/revoke, registry token rotation, cryptography 50.0.0, auth session pruning, Library settings scaffolding, doc-gate fixes, consent-defer 409, SPA deps, plus the accumulated Unreleased items). Tag + GitHub release follow the merge.
Summary by CodeRabbit
New Features
Bug Fixes