feat(telos): integrate Telos SQLite for reads and promote

[dimakis]

Summary

• New telos-store.ts: readonly better-sqlite3 reader for smart_todo.db with proper parent/child tree building
• GET /api/todos now reads directly from Telos SQLite (eliminates Python subprocess for reads)
• POST /api/workload/items/:id/promote falls back to TelosStore when item not in WorkloadStore, bridging Telos SHA256 IDs to Task Board
• 18 new tests covering tree building, child counts, prefix matching, profile listing

Bugs fixed

1. "Item not found" on Promote — Telos items (SHA256 IDs) were not found in WorkloadStore (UUID IDs). Now falls back to TelosStore.
2. SUB-TASKS 0/9 — The old Python path fetched only active/acknowledged items then tried to backfill completed children. TelosStore fetches ALL children regardless of status before counting.

Companion PR: dimakis/mgmt#202 (goal_id column + --set-goal CLI)

Test plan

• 18/18 telos-store tests pass
• 25/25 workload-store tests pass (no regressions)
• TypeScript compiles cleanly (no new errors)
• Manual: verify Telos items load with correct child counts
• Manual: verify Promote creates task and sets goalId on Telos item

🤖 Generated with Claude Code

[dimakis]

Centaur Review

Found 8 issue(s) (4 warning).

server/app.ts

Solid TelosStore implementation with good test coverage of the store itself, but the HTTP layer changes bypass Zod validation (schema drift risk), the promote endpoint returns item: null for Telos items which may break frontend callers, and integration tests for the changed endpoints are missing.

• 🟡 regressions (L1700): The TelosStore path bypasses the TodoListResponse Zod validation that the old Python-subprocess path used. TelosItem includes extra fields not in the Zod schema or frontend types: links (array of TelosLink) and contextHints.sessionIds. These extra fields are silently serialized to the client. While extra JSON properties are harmless today, skipping validation means the endpoint has no runtime guarantee the shape matches the contract. Consider either (a) passing the result through TodoListResponse.parse() to strip/validate, or (b) omitting links from the TelosItem shape (or mapping TelosItem → TodoItem before returning). [fixable]
• 🟡 unsafe_assumptions (L1920): execFileAsync('python3', [TODO_SCRIPT, '--set-goal', telosItem.id, task.id]) invokes a --set-goal CLI flag on todo_api.py that doesn't exist in this repo and can't be verified. If the Python script doesn't support this flag yet, every promote-from-Telos call will silently fail (the .catch swallows the error with a log.warn). The fire-and-forget pattern is fine, but a comment noting the dependency or a check in the Python side would reduce surprise.
• 🟡 regressions (L1930): When promoting a Telos item, the response is { task, item: null }. The old (workload) path returns { task, item: updatedItem } with the updated item. Frontend callers of this endpoint that read item from the response to update local state will get null for Telos promotions. Verify the frontend handles item: null gracefully — if it doesn't, this is a runtime error. [fixable]
• 🔵 unsafe_assumptions (L275): TELOS_DB path is hardcoded to join(BASE_REPO, 'command_center', 'data', 'smart_todo.db'). If BASE_REPO is empty (env var unset), this resolves to a relative path like command_center/data/smart_todo.db. The existsSync guard prevents a crash, but the path assumption couples this to the mgmt repo's internal directory layout. Consider making it configurable via .mitzo.json or env var. [fixable]

server/__tests__/telos-store.test.ts

Solid TelosStore implementation with good test coverage of the store itself, but the HTTP layer changes bypass Zod validation (schema drift risk), the promote endpoint returns item: null for Telos items which may break frontend callers, and integration tests for the changed endpoints are missing.

• 🟡 missing_tests: The TelosStore unit tests are thorough, but there are no integration tests for the changed HTTP endpoints: (1) GET /api/todos with TelosStore present vs absent, (2) GET /api/todos?refresh=true triggering Python then reading TelosStore, (3) POST /api/workload/items/:id/promote falling through from WorkloadStore to TelosStore. The existing todos.test.ts and workload-routes.test.ts files should be updated to cover the new TelosStore paths. [fixable]

server/telos-store.ts

Solid TelosStore implementation with good test coverage of the store itself, but the HTTP layer changes bypass Zod validation (schema drift risk), the promote endpoint returns item: null for Telos items which may break frontend callers, and integration tests for the changed endpoints are missing.

• 🔵 bugs (L123): parseContextHints null path: return { ...EMPTY_HINTS, sessionIds: [] } — the sessionIds: [] override is redundant since EMPTY_HINTS already has sessionIds: []. The shallow spread also shares the same frozen array references for all other fields (repos, paths, etc.), though this is safe in practice since the arrays are empty and the returned object is a new reference. Minor nit. [fixable]
• 🔵 style (L1): The file-level JSDoc block (11 lines) is longer than the project convention of 'default to writing no comments, one short line max'. The information about WAL concurrency and write delegation is useful but could be a single line. [fixable]
• 🔵 bugs (L357): loadLinksForItems queries sqlite_master on every call to check if the links table exists. Since the DB is opened readonly and the schema won't change during the process lifetime, this check could be cached in the constructor (like hasGoalId) to avoid a query per request. [fixable]

[dimakis]

🟡 regressions: The TelosStore path bypasses the TodoListResponse Zod validation that the old Python-subprocess path used. TelosItem includes extra fields not in the Zod schema or frontend types: links (array of TelosLink) and contextHints.sessionIds. These extra fields are silently serialized to the client. While extra JSON properties are harmless today, skipping validation means the endpoint has no runtime guarantee the shape matches the contract. Consider either (a) passing the result through TodoListResponse.parse() to strip/validate, or (b) omitting links from the TelosItem shape (or mapping TelosItem → TodoItem before returning). [fixable]

[dimakis]

🟡 unsafe_assumptions: execFileAsync('python3', [TODO_SCRIPT, '--set-goal', telosItem.id, task.id]) invokes a --set-goal CLI flag on todo_api.py that doesn't exist in this repo and can't be verified. If the Python script doesn't support this flag yet, every promote-from-Telos call will silently fail (the .catch swallows the error with a log.warn). The fire-and-forget pattern is fine, but a comment noting the dependency or a check in the Python side would reduce surprise.

[dimakis]

🟡 regressions: When promoting a Telos item, the response is { task, item: null }. The old (workload) path returns { task, item: updatedItem } with the updated item. Frontend callers of this endpoint that read item from the response to update local state will get null for Telos promotions. Verify the frontend handles item: null gracefully — if it doesn't, this is a runtime error. [fixable]

[dimakis]

🔵 unsafe_assumptions: TELOS_DB path is hardcoded to join(BASE_REPO, 'command_center', 'data', 'smart_todo.db'). If BASE_REPO is empty (env var unset), this resolves to a relative path like command_center/data/smart_todo.db. The existsSync guard prevents a crash, but the path assumption couples this to the mgmt repo's internal directory layout. Consider making it configurable via .mitzo.json or env var. [fixable]

[dimakis]

🔵 bugs: parseContextHints null path: return { ...EMPTY_HINTS, sessionIds: [] } — the sessionIds: [] override is redundant since EMPTY_HINTS already has sessionIds: []. The shallow spread also shares the same frozen array references for all other fields (repos, paths, etc.), though this is safe in practice since the arrays are empty and the returned object is a new reference. Minor nit. [fixable]

[dimakis]

🔵 style: The file-level JSDoc block (11 lines) is longer than the project convention of 'default to writing no comments, one short line max'. The information about WAL concurrency and write delegation is useful but could be a single line. [fixable]

[dimakis]

🔵 bugs: loadLinksForItems queries sqlite_master on every call to check if the links table exists. Since the DB is opened readonly and the schema won't change during the process lifetime, this check could be cached in the constructor (like hasGoalId) to avoid a query per request. [fixable]

[dimakis]

Centaur Review

Found 9 issue(s) (1 critical) (5 warning).

server/app.ts

Solid read-only store implementation with good test coverage of the store itself, but the /api/todos endpoint sends TelosItem objects to the frontend without schema validation (unlike the Python fallback path), and the refresh-without-profile behavior changed. The promote endpoint's fire-and-forget bridge to Python creates a consistency gap.

• 🔴 regressions: The /api/todos TelosStore path returns TelosItem objects directly (which include a links field and sessionIds in contextHints), bypassing the TodoListResponse Zod schema validation that the Python fallback path uses. The frontend TodoItem type (frontend/src/types/todo.ts) has no links field — this won't break anything today since the frontend ignores unknown fields, but the sessionIds field in TelosContextHints is not present in the frontend's TodoContextHints, creating a silent type mismatch. More importantly, the TelosStore path sends unvalidated data to the client, diverging from the pattern used in the fallback path. [fixable]
• 🟡 regressions: The GET /api/todos?refresh=true flow changed behavior: previously, a refresh without a profile (refresh=true but no profile param) would fall into the --list branch. Now, the refresh-then-read path requires BOTH refresh AND profile to be set (if (refresh && profile && existsSync(...))). A client calling ?refresh=true without a profile will skip the Python refresh entirely and go straight to the TelosStore read, returning potentially stale data. The old code handled refresh without profile (it would run --refresh without --profile). [fixable]
• 🟡 unsafe_assumptions: The promote endpoint's Telos path uses fire-and-forget execFileAsync to set goal_id via Python (--set-goal). If this fails, the task is created in taskStore but the Telos item's goal_id is never set — the frontend will show the item as still promotable. There's no reconciliation mechanism, and the warning log is the only signal. The item: null response also means the frontend can't update the item's goalId optimistically.

server/telos-store.ts

Solid read-only store implementation with good test coverage of the store itself, but the /api/todos endpoint sends TelosItem objects to the frontend without schema validation (unlike the Python fallback path), and the refresh-without-profile behavior changed. The promote endpoint's fire-and-forget bridge to Python creates a consistency gap.

• 🟡 bugs: In getItem(), when loading children, sources and links for child items are fetched separately (childSourceMap, childLinkMap) and then merged into the parent's sourceMap/linkMap. However, if the parent item's ID also appears in childIds (which can't happen since parent_id != id due to foreign key), there's no issue. But the bigger concern is that fetchAllChildren only fetches one level deep. The buildTree method supports arbitrary nesting, but fetchAllChildren only queries direct children. If items nest deeper than parent→child (the comment says they don't, but the schema has no constraint preventing it), deeper descendants would be silently dropped. [fixable]
• 🔵 style: The file has a 12-line module-level JSDoc comment block. Per project conventions: 'default to writing no comments' and 'never write multi-paragraph docstrings or multi-line comment blocks'. The listItems and getItem methods also have multi-line JSDoc blocks. These could be removed since the method names and signatures are self-explanatory. [fixable]
• 🔵 style: The EMPTY_HINTS frozen object is spread with { ...EMPTY_HINTS, sessionIds: [] } in two places in parseContextHints, but EMPTY_HINTS already has sessionIds: []. The spread override is redundant. [fixable]
• 🔵 bugs: In loadLinksForItems(), the sqlite_master check runs on every call, even when invoked multiple times within the same listItems() call (once for root items, potentially again for children in getItem). This could be cached as a class-level flag like hasGoalId is. [fixable]

server/__tests__/telos-store.test.ts

Solid read-only store implementation with good test coverage of the store itself, but the /api/todos endpoint sends TelosItem objects to the frontend without schema validation (unlike the Python fallback path), and the refresh-without-profile behavior changed. The promote endpoint's fire-and-forget bridge to Python creates a consistency gap.

• 🟡 missing_tests: No tests for the links table existence check in loadLinksForItems() — the method checks sqlite_master for the links table to handle pre-migration DBs. This defensive code path (no links table) is untested. Also no test for the hasGoalId migration check (DB without goal_id column). [fixable]
• 🟡 missing_tests: No integration tests for the modified /api/todos endpoint or the /api/workload/items/:id/promote Telos fallback path in app.ts. The test file only covers the TelosStore class in isolation. The endpoint behavior changes (refresh flow, dual-source promote, fire-and-forget Python bridge) are untested. [fixable]

[dimakis]

Centaur Review

LGTM — no issues found.

[dimakis]

Centaur Review

Found 7 issue(s) (3 warning).

server/telos-store.ts

Solid read-only store integration with good test coverage and proper readonly DB handling. Main concerns: shallow-freeze sharing mutable arrays in EMPTY_HINTS, missing WS broadcast for Telos promotes, and fire-and-forget goal-setting risking duplicate tasks on retry.

• 🟡 bugs (L113): EMPTY_HINTS is shallowly frozen via Object.freeze, but { ...EMPTY_HINTS } creates a shallow copy whose array properties (repos, paths, etc.) are shared references to the same mutable array objects. If any consumer mutates e.g. hints.repos.push(...), it corrupts the template for all future fallback calls. Either deep-freeze the nested arrays or return fresh arrays in parseContextHints's fallback path (e.g. repos: [], paths: [], ...). [fixable]
• 🔵 style (L258): The getItem prefix match query WHERE substr(id, 1, ?) = ? cannot use indexes and triggers a full table scan. An equivalent WHERE id LIKE ? || '%' would at least allow a prefix index scan on the id primary key. Minor for the current data volume but worth noting. [fixable]

server/app.ts

Solid read-only store integration with good test coverage and proper readonly DB handling. Main concerns: shallow-freeze sharing mutable arrays in EMPTY_HINTS, missing WS broadcast for Telos promotes, and fire-and-forget goal-setting risking duplicate tasks on retry.

• 🟡 regressions (L1935): The Telos promote path no longer broadcasts onWorkloadBroadcast (the WorkloadStore path still does at line 1897). Other connected clients relying on workload_item_updated WS events to update the todo list's goalId badge in real-time won't see the change until they refresh. The mappedItem is already constructed — consider broadcasting it or documenting the omission as intentional. [fixable]
• 🟡 regressions (L1929): The mappedItem returned for Telos promotes has only {id, title, status, goalId}, a much smaller shape than the full TodoItem/WorkloadItem the WorkloadStore path returns. While the current frontend only reads result.task.id and navigates away, any future consumer expecting fields like urgency, starred, contextHints, sources, ageDays, or profile on the response item will get undefined. Consider returning a fuller shape or documenting this as a minimal-response contract. [fixable]
• 🔵 unsafe_assumptions (L1919): The --set-goal Python call is fire-and-forget: if it fails, the TaskStore has a new task but the Telos DB still shows goal_id = NULL. A subsequent promote attempt on the same item would create a duplicate task. The .catch logs the error but the user sees success. Consider returning the Telos item's existing goalId in the response if it's already set, or checking telosItem.goalId before creating a new task. [fixable]

server/__tests__/telos-store.test.ts

Solid read-only store integration with good test coverage and proper readonly DB handling. Main concerns: shallow-freeze sharing mutable arrays in EMPTY_HINTS, missing WS broadcast for Telos promotes, and fire-and-forget goal-setting risking duplicate tasks on retry.

• 🔵 missing_tests: No test covers the case where a child item matches the root status filter AND its parent also matches (both 'active'). This exercises a key correctness property of buildTree — the child must appear nested under the parent, not as a duplicate root. Adding a test that inserts an active parent with an active child and asserts items.length === 1 with items[0].children.length === 1 would lock this down. [fixable]
• 🔵 missing_tests: No test for listItems with an empty statuses array ([]). The current code skips the status WHERE clause when statuses is empty, returning all items regardless of status. If this is intentional, a test would document the behavior; if not, it's a latent bug. [fixable]

[dimakis]

🟡 bugs: EMPTY_HINTS is shallowly frozen via Object.freeze, but { ...EMPTY_HINTS } creates a shallow copy whose array properties (repos, paths, etc.) are shared references to the same mutable array objects. If any consumer mutates e.g. hints.repos.push(...), it corrupts the template for all future fallback calls. Either deep-freeze the nested arrays or return fresh arrays in parseContextHints's fallback path (e.g. repos: [], paths: [], ...). [fixable]

[dimakis]

🔵 style: The getItem prefix match query WHERE substr(id, 1, ?) = ? cannot use indexes and triggers a full table scan. An equivalent WHERE id LIKE ? || '%' would at least allow a prefix index scan on the id primary key. Minor for the current data volume but worth noting. [fixable]

[dimakis]

🟡 regressions: The Telos promote path no longer broadcasts onWorkloadBroadcast (the WorkloadStore path still does at line 1897). Other connected clients relying on workload_item_updated WS events to update the todo list's goalId badge in real-time won't see the change until they refresh. The mappedItem is already constructed — consider broadcasting it or documenting the omission as intentional. [fixable]

[dimakis]

🟡 regressions: The mappedItem returned for Telos promotes has only {id, title, status, goalId}, a much smaller shape than the full TodoItem/WorkloadItem the WorkloadStore path returns. While the current frontend only reads result.task.id and navigates away, any future consumer expecting fields like urgency, starred, contextHints, sources, ageDays, or profile on the response item will get undefined. Consider returning a fuller shape or documenting this as a minimal-response contract. [fixable]

[dimakis]

🔵 unsafe_assumptions: The --set-goal Python call is fire-and-forget: if it fails, the TaskStore has a new task but the Telos DB still shows goal_id = NULL. A subsequent promote attempt on the same item would create a duplicate task. The .catch logs the error but the user sees success. Consider returning the Telos item's existing goalId in the response if it's already set, or checking telosItem.goalId before creating a new task. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (2 warning).

server/__tests__/telos-store.test.ts

Solid integration that replaces Python subprocess reads with direct SQLite access. Main concerns are missing WS broadcast for Telos-promoted items and missing test coverage for schema evolution guards.

• 🟡 missing_tests: Schema evolution guards hasGoalId and hasLinksTable (telos-store.ts lines 176–185) are not tested. These paths defensively handle older DBs that lack the goal_id column or links table. A test with a DB created without those schema elements would confirm the fallback behavior works. [fixable]

server/app.ts

Solid integration that replaces Python subprocess reads with direct SQLite access. Main concerns are missing WS broadcast for Telos-promoted items and missing test coverage for schema evolution guards.

• 🟡 regressions: The Telos promote path (starting around the telosItem fallback) does not emit onWorkloadBroadcast after promotion. The WorkloadStore path emits { type: 'workload_item_updated', item: updatedItem } so other connected clients see the goalId update in real-time. Telos-promoted items won't get this broadcast, so other open tabs/devices won't reflect the promotion until the next full fetch. [fixable]
• 🔵 regressions: The Python fallback path for /api/todos (when telosStore is null) removed the log.warn('todo API returned unexpected shape', ...) that previously fired on Zod validation failure. The ternary parsed.success ? parsed.data : { profiles: [], items: [] } silently swallows bad data, reducing observability for debugging the Python script. [fixable]
• 🔵 unsafe_assumptions: The --set-goal Python call in the Telos promote path is fire-and-forget and depends on a CLI flag from an unmerged PR (mgmt PR #202, as noted in the comment). If the Python script doesn't support this flag, goal_id is never persisted in the Telos DB — the TaskStore task will exist but the Telos item won't link to it, so on next page load the item appears un-promoted. Consider logging at info level (not just warn on failure) to make this observable, or gating the call on a feature check. [fixable]

server/telos-store.ts

Solid integration that replaces Python subprocess reads with direct SQLite access. Main concerns are missing WS broadcast for Telos-promoted items and missing test coverage for schema evolution guards.

• 🔵 style: The fetchAllChildren comment says 'Single level — items in this repo don't nest deeper than parent→child'. If a child itself has children, they would be silently dropped. This is fine as a documented constraint, but consider adding a clarifying note that childCount/completedChildCount only reflect direct children, or adding a recursive fetch if deeper nesting is possible in the future.

[dimakis]

Centaur Review

Found 7 issue(s) (1 critical) (3 warning).

server/app.ts

Solid read-only SQLite integration with good defensive coding (schema checks, busy_timeout, graceful fallback), but the promote endpoint has a type-shape mismatch when broadcasting TelosItems as WorkloadItems and a race condition on the duplicate-prevention guard. Both are fixable without major rework.

• 🔴 bugs (L1946): The promote endpoint broadcasts workload_item_updated with a TelosItem (has summary, ageDays, links, children) but the frontend protocol-parser.ts casts this as WorkloadItem (expects title, snippet, snoozedUntil, clusterId, createdAt, updatedAt). The updateWorkloadItem() function in @mitzo/client matches by item.id and since the Telos item wouldn't exist in the workload slice, it's a no-op — but downstream consumers that read item.title from the broadcast will get undefined instead of the actual summary. Consider either (a) broadcasting a different event type for Telos promotes, or (b) mapping the TelosItem to a WorkloadItem shape before broadcasting. [fixable]
• 🟡 bugs (L1907): The 'already promoted' guard (if (telosItem.goalId)) reads from a readonly DB connection, but goal_id is written by a fire-and-forget Python subprocess (--set-goal). If two promote requests arrive before the first Python process commits, both will see goalId === null and create duplicate tasks. Consider using an in-memory Set or the taskStore itself (check for existing task with matching Telos item ID in annotations) to prevent duplicates. [fixable]
• 🟡 regressions (L1699): The TelosStore path in /api/todos skips TodoListResponse.safeParse() validation that the Python fallback path performs (line 1688). TelosItem includes a links field not present in the TodoItemSchema Zod schema, and TelosContextHints has a sessionIds field absent from TodoContextHintsSchema. While extra fields pass through JSON.stringify harmlessly, the inconsistency means any future strict-mode or .strict() addition to the schema would break this path silently. Consider running TodoListResponse.safeParse() on the TelosStore response too, or updating the schema. [fixable]
• 🟡 missing_tests (L1900): The promote endpoint's TelosStore fallback path (lines 1900–1947) has no integration test coverage. The existing workload-routes.test.ts tests only cover WorkloadStore promotes. Key scenarios missing: (1) promote a Telos item successfully, (2) promote an already-promoted Telos item returns 200 with existing task, (3) item not found in either store returns 404. [fixable]
• 🔵 missing_tests (L1697): The /api/todos TelosStore path (lines 1696–1703) is not covered by todos.test.ts. That test file only exercises the Python fallback path. Adding a test that mocks TelosStore presence would verify the primary read path. [fixable]
• 🔵 style (L1874): The WorkloadStore promote logic (lines 1874–1896) and TelosStore promote logic (lines 1912–1946) duplicate the description-building pattern (descParts + hintsWithValues). Consider extracting a shared buildPromoteDescription(contextHints, description?) helper to reduce duplication. [fixable]

server/telos-store.ts

Solid read-only SQLite integration with good defensive coding (schema checks, busy_timeout, graceful fallback), but the promote endpoint has a type-shape mismatch when broadcasting TelosItems as WorkloadItems and a race condition on the duplicate-prevention guard. Both are fixable without major rework.

• 🔵 unsafe_assumptions (L316): fetchAllChildren only fetches one level deep (items whose parent_id is in the given set). The inline comment acknowledges this ('items in this repo don't nest deeper than parent→child'), but this is a data assumption not enforced by the schema. If a grandchild exists, it would be silently dropped. This is acceptable if the Telos schema guarantees max depth 2, but worth a note in case that invariant changes.

[dimakis]

🔴 bugs: The promote endpoint broadcasts workload_item_updated with a TelosItem (has summary, ageDays, links, children) but the frontend protocol-parser.ts casts this as WorkloadItem (expects title, snippet, snoozedUntil, clusterId, createdAt, updatedAt). The updateWorkloadItem() function in @mitzo/client matches by item.id and since the Telos item wouldn't exist in the workload slice, it's a no-op — but downstream consumers that read item.title from the broadcast will get undefined instead of the actual summary. Consider either (a) broadcasting a different event type for Telos promotes, or (b) mapping the TelosItem to a WorkloadItem shape before broadcasting. [fixable]

[dimakis]

🟡 bugs: The 'already promoted' guard (if (telosItem.goalId)) reads from a readonly DB connection, but goal_id is written by a fire-and-forget Python subprocess (--set-goal). If two promote requests arrive before the first Python process commits, both will see goalId === null and create duplicate tasks. Consider using an in-memory Set or the taskStore itself (check for existing task with matching Telos item ID in annotations) to prevent duplicates. [fixable]

[dimakis]

🟡 regressions: The TelosStore path in /api/todos skips TodoListResponse.safeParse() validation that the Python fallback path performs (line 1688). TelosItem includes a links field not present in the TodoItemSchema Zod schema, and TelosContextHints has a sessionIds field absent from TodoContextHintsSchema. While extra fields pass through JSON.stringify harmlessly, the inconsistency means any future strict-mode or .strict() addition to the schema would break this path silently. Consider running TodoListResponse.safeParse() on the TelosStore response too, or updating the schema. [fixable]

[dimakis]

🟡 missing_tests: The promote endpoint's TelosStore fallback path (lines 1900–1947) has no integration test coverage. The existing workload-routes.test.ts tests only cover WorkloadStore promotes. Key scenarios missing: (1) promote a Telos item successfully, (2) promote an already-promoted Telos item returns 200 with existing task, (3) item not found in either store returns 404. [fixable]

[dimakis]

🔵 missing_tests: The /api/todos TelosStore path (lines 1696–1703) is not covered by todos.test.ts. That test file only exercises the Python fallback path. Adding a test that mocks TelosStore presence would verify the primary read path. [fixable]

[dimakis]

🔵 style: The WorkloadStore promote logic (lines 1874–1896) and TelosStore promote logic (lines 1912–1946) duplicate the description-building pattern (descParts + hintsWithValues). Consider extracting a shared buildPromoteDescription(contextHints, description?) helper to reduce duplication. [fixable]

[dimakis]

🔵 unsafe_assumptions: fetchAllChildren only fetches one level deep (items whose parent_id is in the given set). The inline comment acknowledges this ('items in this repo don't nest deeper than parent→child'), but this is a data assumption not enforced by the schema. If a grandchild exists, it would be silently dropped. This is acceptable if the Telos schema guarantees max depth 2, but worth a note in case that invariant changes.

[dimakis]

Centaur Review

Found 6 issue(s) (3 warning).

server/app.ts

Well-structured readonly SQLite integration with good test coverage for the store itself. Main concerns: partial task stub in the already-promoted guard, missing schema-evolution test coverage, and duplicated promote logic. The workload broadcast shape mismatch appears already fixed in the latest branch commit.

• 🟡 bugs (L1946): Broadcasting workload_item_updated with a TelosItem (has summary, TelosSource.type) into the workload store that expects WorkloadItem (has title, WorkloadSource.sourceType). The shape mismatch means updateWorkloadItem silently no-ops (IDs won't match workload items), but the broadcast is misleading dead code. NOTE: commit edf7ba9 on the branch already removes this line — confirm it's included in the PR diff. [fixable]
• 🟡 bugs (L1909): When a Telos item is already promoted but the task was deleted from TaskStore, existingTask ?? { id: telosItem.goalId } returns a minimal stub { id } instead of a full Task object. The frontend promote handler likely expects a complete task shape (title, status, etc.). Consider returning 404 or re-creating the task when taskStore.get() returns undefined. [fixable]
• 🔵 style (L1878): The description-building logic (descParts, hintsWithValues) is duplicated between the WorkloadStore path (lines 1875–1881) and the TelosStore path (lines 1913–1919). Extract a helper like buildPromoteDescription(body, contextHints, taskHint) to reduce duplication. [fixable]

server/__tests__/telos-store.test.ts

Well-structured readonly SQLite integration with good test coverage for the store itself. Main concerns: partial task stub in the already-promoted guard, missing schema-evolution test coverage, and duplicated promote logic. The workload broadcast shape mismatch appears already fixed in the latest branch commit.

• 🟡 missing_tests: The TelosStore constructor explicitly handles schema evolution (hasGoalId, hasLinksTable flags) but no tests exercise these paths. Add test cases with a DB that lacks the goal_id column and/or the links table to verify graceful degradation. [fixable]
• 🔵 missing_tests: No integration test for the Telos promote path in /api/workload/items/:id/promote — specifically the idempotent guard (already-promoted returns 200), the --set-goal fire-and-forget bridging, and the fallback from WorkloadStore to TelosStore. The existing workload-routes.test.ts only tests the WorkloadStore path. [fixable]

server/telos-store.ts

Well-structured readonly SQLite integration with good test coverage for the store itself. Main concerns: partial task stub in the already-promoted guard, missing schema-evolution test coverage, and duplicated promote logic. The workload broadcast shape mismatch appears already fixed in the latest branch commit.

• 🔵 unsafe_assumptions (L259): getItem('') would match ALL items: SQLite's substr(id, 1, 0) returns '', and '' = '' is true for every row. If the prefix has multiple matches, it returns null (safe), but an empty string on a single-item DB would return that item. Consider guarding against empty/very short ID prefixes (e.g., if (id.length < 4) return null). [fixable]

[dimakis]

🟡 bugs: Broadcasting workload_item_updated with a TelosItem (has summary, TelosSource.type) into the workload store that expects WorkloadItem (has title, WorkloadSource.sourceType). The shape mismatch means updateWorkloadItem silently no-ops (IDs won't match workload items), but the broadcast is misleading dead code. NOTE: commit edf7ba9 on the branch already removes this line — confirm it's included in the PR diff. [fixable]

[dimakis]

🟡 bugs: When a Telos item is already promoted but the task was deleted from TaskStore, existingTask ?? { id: telosItem.goalId } returns a minimal stub { id } instead of a full Task object. The frontend promote handler likely expects a complete task shape (title, status, etc.). Consider returning 404 or re-creating the task when taskStore.get() returns undefined. [fixable]

[dimakis]

🔵 style: The description-building logic (descParts, hintsWithValues) is duplicated between the WorkloadStore path (lines 1875–1881) and the TelosStore path (lines 1913–1919). Extract a helper like buildPromoteDescription(body, contextHints, taskHint) to reduce duplication. [fixable]

[dimakis]

🔵 unsafe_assumptions: getItem('') would match ALL items: SQLite's substr(id, 1, 0) returns '', and '' = '' is true for every row. If the prefix has multiple matches, it returns null (safe), but an empty string on a single-item DB would return that item. Consider guarding against empty/very short ID prefixes (e.g., if (id.length < 4) return null). [fixable]

[dimakis]

Centaur Review

Found 6 issue(s) (3 warning).

server/app.ts

Solid readonly store with good test coverage and defensive schema checks, but the Telos promote path has a race condition from fire-and-forget goal_id write-back and lacks integration tests.

• 🟡 bugs (L1907): Race condition: the idempotency guard (if (telosItem.goalId)) relies on the fire-and-forget Python --set-goal call completing before any retry. Since the TelosStore is readonly and the Python write-back is async, a fast double-promote creates duplicate tasks — both calls see goalId: null, both create tasks, and goal_id is set to whichever Python call finishes last. Consider awaiting the Python call (like the refresh path does) or adding an in-memory promote lock keyed by item ID. [fixable]
• 🟡 regressions (L1697): The /api/todos endpoint now returns TelosItem[] directly without Zod validation via TodoListResponse. The previous code always validated through TodoListResponse.safeParse(). TelosItem includes a links field and contextHints.sessionIds that aren't in the TodoListResponse schema or the frontend's TodoItem type. The extra fields are harmless in JSON, but the endpoint now bypasses the schema contract entirely — any future TelosStore shape change will pass through to the frontend unvalidated. [fixable]
• 🟡 missing_tests (L1899): The Telos promote path (lines 1899–1946) has no integration test coverage. The existing workload-routes.test.ts promote tests only exercise the WorkloadStore path. Missing cases: (1) promote a Telos item when WorkloadStore has no match, (2) the already-promoted guard returning 200, (3) the fallback to 404 when both stores miss, (4) the fire-and-forget Python --set-goal call. [fixable]
• 🔵 bugs (L1909): When a Telos item is already promoted but the linked task has been deleted from taskStore, this returns { task: { id: telosItem.goalId }, item: telosItem } — a stub object with only id and no other task fields. Callers/frontend expecting a full Task shape (status, title, etc.) will get undefined fields. Consider returning 409 or re-creating the task instead of returning a partial object. [fixable]
• 🔵 style (L1913): The description-building logic (descParts + hintsWithValues assembly) is duplicated verbatim between the WorkloadStore path (lines 1875–1881) and the TelosStore path (lines 1913–1919). Extract to a helper to reduce the duplication, especially since the filter conditions differ subtly (sessionIds is excluded only in the Telos path). [fixable]

server/__tests__/telos-store.test.ts

Solid readonly store with good test coverage and defensive schema checks, but the Telos promote path has a race condition from fire-and-forget goal_id write-back and lacks integration tests.

• 🔵 missing_tests: No test for getItem with an empty string input. SQLite substr(id, 1, 0) = '' matches all rows, but the rows.length === 1 guard should return null. Also no test for listItems when a child's parent doesn't match the status filter (orphaned child becomes a root item). [fixable]

[dimakis]

🟡 bugs: Race condition: the idempotency guard (if (telosItem.goalId)) relies on the fire-and-forget Python --set-goal call completing before any retry. Since the TelosStore is readonly and the Python write-back is async, a fast double-promote creates duplicate tasks — both calls see goalId: null, both create tasks, and goal_id is set to whichever Python call finishes last. Consider awaiting the Python call (like the refresh path does) or adding an in-memory promote lock keyed by item ID. [fixable]

[dimakis]

🟡 regressions: The /api/todos endpoint now returns TelosItem[] directly without Zod validation via TodoListResponse. The previous code always validated through TodoListResponse.safeParse(). TelosItem includes a links field and contextHints.sessionIds that aren't in the TodoListResponse schema or the frontend's TodoItem type. The extra fields are harmless in JSON, but the endpoint now bypasses the schema contract entirely — any future TelosStore shape change will pass through to the frontend unvalidated. [fixable]

[dimakis]

🟡 missing_tests: The Telos promote path (lines 1899–1946) has no integration test coverage. The existing workload-routes.test.ts promote tests only exercise the WorkloadStore path. Missing cases: (1) promote a Telos item when WorkloadStore has no match, (2) the already-promoted guard returning 200, (3) the fallback to 404 when both stores miss, (4) the fire-and-forget Python --set-goal call. [fixable]

[dimakis]

🔵 bugs: When a Telos item is already promoted but the linked task has been deleted from taskStore, this returns { task: { id: telosItem.goalId }, item: telosItem } — a stub object with only id and no other task fields. Callers/frontend expecting a full Task shape (status, title, etc.) will get undefined fields. Consider returning 409 or re-creating the task instead of returning a partial object. [fixable]

[dimakis]

🔵 style: The description-building logic (descParts + hintsWithValues assembly) is duplicated verbatim between the WorkloadStore path (lines 1875–1881) and the TelosStore path (lines 1913–1919). Extract to a helper to reduce the duplication, especially since the filter conditions differ subtly (sessionIds is excluded only in the Telos path). [fixable]

[dimakis]

Centaur Review

Found 9 issue(s) (5 warning).

server/app.ts

Solid read-only store with good test coverage for the core class, but the route-level integration (dual-source promote, response validation bypass, fire-and-forget goal writes) has gaps in both testing and error handling that could cause subtle UX inconsistencies.

• 🟡 regressions (L1700): The TelosStore path in /api/todos bypasses TodoListResponse.safeParse() validation that was applied to Python output. If TelosStore returns malformed data (e.g., null urgency, missing fields), it goes straight to the client unvalidated. Consider wrapping with TodoListResponse.safeParse({ profiles, items }) for consistency, or at minimum document why validation is skipped. [fixable]
• 🟡 regressions (L1658): The old GET /api/todos handler checked existsSync(TODO_SCRIPT) and returned empty early for all cases (list + refresh). The new code only checks TODO_SCRIPT existence for the refresh path but not before calling it. If refresh=true and profile is set but TODO_SCRIPT is missing, the code correctly guards with existsSync(TODO_SCRIPT) in the if condition, so this is fine. However, when telosStore is null and falling back to Python, the --list fallback also correctly guards. No regression here — disregard.
• 🟡 bugs (L1905): In the Telos promote path, the --set-goal Python call is fire-and-forget. If it fails (and it's logged), the response still returns goalId: task.id to the frontend. On the next GET /api/todos call, the item will come back from TelosStore without goalId (since the DB write failed), causing the frontend to show the Promote button again. This is a known trade-off (documented as 'requires mgmt PR #202'), but the UX inconsistency could be confusing.
• 🟡 bugs (L1893): The duplicate-promote guard (if (telosItem.goalId)) returns the existing task via taskStore.get(telosItem.goalId). If the task was deleted from TaskStore but goal_id still exists in Telos DB, existingTask will be null, and the response becomes { task: { id: telosItem.goalId }, item: telosItem } — a partial task object with only id. Callers expecting a full task shape will get unexpected results. [fixable]
• 🔵 style (L1868): The promote endpoint now has two near-identical blocks for building descParts and hintsWithValues — one for WorkloadStore items, one for TelosStore items. The only differences are field names (title vs summary, sourceType vs type, extra sessionIds filter). Consider extracting a shared helper to reduce duplication. [fixable]
• 🔵 regressions (L1898): The Telos promote path does not call onWorkloadBroadcast (unlike the WorkloadStore path). This is intentional since it's not a workload item, but frontend components listening for workload_item_updated won't get notified when a Telos item is promoted. If the frontend todo list needs to refresh after promote, it may need a separate broadcast or the client needs to re-fetch. [fixable]

server/__tests__/telos-store.test.ts

Solid read-only store with good test coverage for the core class, but the route-level integration (dual-source promote, response validation bypass, fire-and-forget goal writes) has gaps in both testing and error handling that could cause subtle UX inconsistencies.

• 🟡 missing_tests: No integration tests for the modified /api/todos or /api/workload/items/:id/promote endpoints. The unit tests for TelosStore are thorough, but the route handler changes (dual-source promote, TelosStore-first reads, refresh-then-read flow) are untested. Key missing scenario: promote endpoint falling through from WorkloadStore miss to TelosStore hit. [fixable]
• 🔵 missing_tests: No test for TelosStore behavior when goal_id column is missing from the schema (the hasGoalId feature flag path). The constructor caches this check, but no test verifies that goalId is null when the column doesn't exist. [fixable]

server/telos-store.ts

Solid read-only store with good test coverage for the core class, but the route-level integration (dual-source promote, response validation bypass, fire-and-forget goal writes) has gaps in both testing and error handling that could cause subtle UX inconsistencies.

• 🔵 unsafe_assumptions (L241): The prefix-match query SELECT * FROM items WHERE substr(id, 1, ?) = ? does a full table scan — it can't use the primary key index. For a small Telos DB this is fine, but if the table grows, consider WHERE id >= ? AND id < ? (range scan on the index) or WHERE id LIKE ? || '%' with appropriate escaping. [fixable]

[dimakis]

🟡 regressions: The TelosStore path in /api/todos bypasses TodoListResponse.safeParse() validation that was applied to Python output. If TelosStore returns malformed data (e.g., null urgency, missing fields), it goes straight to the client unvalidated. Consider wrapping with TodoListResponse.safeParse({ profiles, items }) for consistency, or at minimum document why validation is skipped. [fixable]

[dimakis]

🟡 regressions: The old GET /api/todos handler checked existsSync(TODO_SCRIPT) and returned empty early for all cases (list + refresh). The new code only checks TODO_SCRIPT existence for the refresh path but not before calling it. If refresh=true and profile is set but TODO_SCRIPT is missing, the code correctly guards with existsSync(TODO_SCRIPT) in the if condition, so this is fine. However, when telosStore is null and falling back to Python, the --list fallback also correctly guards. No regression here — disregard.

[dimakis]

🟡 bugs: In the Telos promote path, the --set-goal Python call is fire-and-forget. If it fails (and it's logged), the response still returns goalId: task.id to the frontend. On the next GET /api/todos call, the item will come back from TelosStore without goalId (since the DB write failed), causing the frontend to show the Promote button again. This is a known trade-off (documented as 'requires mgmt PR #202'), but the UX inconsistency could be confusing.

[dimakis]

🟡 bugs: The duplicate-promote guard (if (telosItem.goalId)) returns the existing task via taskStore.get(telosItem.goalId). If the task was deleted from TaskStore but goal_id still exists in Telos DB, existingTask will be null, and the response becomes { task: { id: telosItem.goalId }, item: telosItem } — a partial task object with only id. Callers expecting a full task shape will get unexpected results. [fixable]

[dimakis]

🔵 style: The promote endpoint now has two near-identical blocks for building descParts and hintsWithValues — one for WorkloadStore items, one for TelosStore items. The only differences are field names (title vs summary, sourceType vs type, extra sessionIds filter). Consider extracting a shared helper to reduce duplication. [fixable]

[dimakis]

🔵 regressions: The Telos promote path does not call onWorkloadBroadcast (unlike the WorkloadStore path). This is intentional since it's not a workload item, but frontend components listening for workload_item_updated won't get notified when a Telos item is promoted. If the frontend todo list needs to refresh after promote, it may need a separate broadcast or the client needs to re-fetch. [fixable]

[dimakis]

🔵 unsafe_assumptions: The prefix-match query SELECT * FROM items WHERE substr(id, 1, ?) = ? does a full table scan — it can't use the primary key index. For a small Telos DB this is fine, but if the table grows, consider WHERE id >= ? AND id < ? (range scan on the index) or WHERE id LIKE ? || '%' with appropriate escaping. [fixable]

[dimakis]

Centaur Review

Found 7 issue(s) (4 warning).

server/app.ts

Well-structured read-only store with good test coverage for the core class, but the app.ts integration has gaps: no duplicate-promote protection until a dependency PR lands, no schema validation on the new TelosStore response path, and missing integration tests for the promote fallback flow.

• 🟡 bugs (L1907): The already promoted guard checks telosItem.goalId, but TelosStore opens the DB readonly. After a fire-and-forget --set-goal call writes goal_id to SQLite, the readonly connection may not see the update (SQLite readonly connections using WAL can see concurrent writes, but only after re-reading — better-sqlite3 will see them on next query in WAL mode, so this likely works, but the guard is fragile since the Python write is fire-and-forget and may fail silently). If --set-goal hasn't been implemented yet (comment says 'requires mgmt PR #202'), the guard will never trigger and duplicate tasks can be created on repeated promotes.
• 🟡 regressions (L1697): The /api/todos response no longer validates against TodoListResponse when using TelosStore. The old path always ran TodoListResponse.safeParse() to ensure response shape conformance. The new TelosStore path returns raw TelosItem[] which includes extra fields (links, contextHints.sessionIds) not in the frontend's TodoItem type. While extra fields are harmless, the loss of schema validation means any future TelosStore shape drift won't be caught at the API boundary. [fixable]
• 🔵 regressions (L1945): The Telos promote path does not call onWorkloadBroadcast (the workload path does at the old line 1895). Frontend components listening for workload_item_updated won't see real-time updates for Telos promotes. This may be intentional (Telos items aren't in WorkloadStore), but the frontend may need a separate broadcast or shared event to update the todo list UI after a promote. [fixable]

server/__tests__/telos-store.test.ts

Well-structured read-only store with good test coverage for the core class, but the app.ts integration has gaps: no duplicate-promote protection until a dependency PR lands, no schema validation on the new TelosStore response path, and missing integration tests for the promote fallback flow.

• 🟡 missing_tests: No integration tests for the promote endpoint's Telos path (/api/workload/items/:id/promote when the item is a Telos item). Key untested behaviors: the already-promoted guard (line 1907), the fire-and-forget --set-goal Python call, and the fallback when telosStore is null. The existing todos.test.ts also doesn't cover the new TelosStore read path. [fixable]

server/telos-store.ts

Well-structured read-only store with good test coverage for the core class, but the app.ts integration has gaps: no duplicate-promote protection until a dependency PR lands, no schema validation on the new TelosStore response path, and missing integration tests for the promote fallback flow.

• 🔵 bugs (L259): substr(id, 1, ?) = ? does a full table scan on every prefix lookup. For a readonly DB this is acceptable at small scale, but consider using id LIKE ? || '%' or id >= ? AND id < ? with a computed upper bound for index-friendly prefix matching if the items table grows. [fixable]
• 🟡 unsafe_assumptions (L322): fetchAllChildren only fetches one level of children. The comment says 'items in this repo don't nest deeper than parent→child', but the DB schema allows arbitrary nesting (parent_id REFERENCES items(id)). If Telos items ever nest 2+ levels, grandchildren won't appear in counts or results. A safer approach would be a recursive CTE or iterative fetch. [fixable]
• 🔵 style (L377): row.status as TelosItem['status'] is an unsafe cast — any string from the DB is accepted. If the DB contains an unexpected status value (e.g. 'dismissed'), it'll silently pass through. Consider validating against the known union or defaulting to a fallback. [fixable]

[dimakis]

🟡 bugs: The already promoted guard checks telosItem.goalId, but TelosStore opens the DB readonly. After a fire-and-forget --set-goal call writes goal_id to SQLite, the readonly connection may not see the update (SQLite readonly connections using WAL can see concurrent writes, but only after re-reading — better-sqlite3 will see them on next query in WAL mode, so this likely works, but the guard is fragile since the Python write is fire-and-forget and may fail silently). If --set-goal hasn't been implemented yet (comment says 'requires mgmt PR #202'), the guard will never trigger and duplicate tasks can be created on repeated promotes.

[dimakis]

🟡 regressions: The /api/todos response no longer validates against TodoListResponse when using TelosStore. The old path always ran TodoListResponse.safeParse() to ensure response shape conformance. The new TelosStore path returns raw TelosItem[] which includes extra fields (links, contextHints.sessionIds) not in the frontend's TodoItem type. While extra fields are harmless, the loss of schema validation means any future TelosStore shape drift won't be caught at the API boundary. [fixable]

[dimakis]

🔵 regressions: The Telos promote path does not call onWorkloadBroadcast (the workload path does at the old line 1895). Frontend components listening for workload_item_updated won't see real-time updates for Telos promotes. This may be intentional (Telos items aren't in WorkloadStore), but the frontend may need a separate broadcast or shared event to update the todo list UI after a promote. [fixable]

[dimakis]

🔵 bugs: substr(id, 1, ?) = ? does a full table scan on every prefix lookup. For a readonly DB this is acceptable at small scale, but consider using id LIKE ? || '%' or id >= ? AND id < ? with a computed upper bound for index-friendly prefix matching if the items table grows. [fixable]

[dimakis]

🟡 unsafe_assumptions: fetchAllChildren only fetches one level of children. The comment says 'items in this repo don't nest deeper than parent→child', but the DB schema allows arbitrary nesting (parent_id REFERENCES items(id)). If Telos items ever nest 2+ levels, grandchildren won't appear in counts or results. A safer approach would be a recursive CTE or iterative fetch. [fixable]

[dimakis]

🔵 style: row.status as TelosItem['status'] is an unsafe cast — any string from the DB is accepted. If the DB contains an unexpected status value (e.g. 'dismissed'), it'll silently pass through. Consider validating against the known union or defaulting to a fallback. [fixable]

[dimakis]

Centaur Review

Found 7 issue(s) (3 warning).

server/app.ts

Solid integration with good test coverage for the store layer. Main concerns: fire-and-forget goal_id persistence creates a duplicate-task risk on retry, and the Telos promote route path lacks route-level test coverage.

• 🟡 bugs (L1949): Fire-and-forget --set-goal via Python means if the subprocess fails (logged but swallowed), goal_id is never persisted in the Telos DB. On the next promote attempt for the same item, telosItem.goalId will still be null (readonly TelosStore re-reads from DB), bypassing the 'already promoted' guard and creating a duplicate task. The promotesInFlight Set only guards within a single in-flight request, not across sequential requests. Consider making the Python call non-fire-and-forget (await it, return 500 on failure) or at minimum logging at error level instead of warn. [fixable]
• 🟡 regressions (L1721): The Telos read path's Zod fallback sends raw items on parse failure ({ profiles, items }) instead of empty arrays. If TelosStore returns data that doesn't match the schema (e.g., a new unexpected status value), the frontend receives unvalidated data without Zod's .default() coercions applied. The Python fallback path correctly returns { profiles: [], items: [] } on parse failure. Consider using the same empty fallback for consistency, or log the parse error to aid debugging. [fixable]
• 🟡 missing_tests (L1916): The Telos branch of the promote endpoint has no route-level tests. The existing workload-routes.test.ts only covers the WorkloadStore branch. Missing coverage: Telos item promote (happy path), already-promoted guard (200 with existing task), deleted-task guard (409), race guard (409 in-flight), and the Python --set-goal bridge path. [fixable]
• 🔵 style (L1661): buildPromoteDescription accepts Record<string, unknown> and both call sites cast contextHints as unknown as Record<string, unknown>. Since both TelosContextHints and the workload ContextHints are known types, the function could accept a typed union (or the shared subset) to avoid the double cast. [fixable]

server/__tests__/telos-store.test.ts

Solid integration with good test coverage for the store layer. Main concerns: fire-and-forget goal_id persistence creates a duplicate-task risk on retry, and the Telos promote route path lacks route-level test coverage.

• 🔵 missing_tests: No test for listItems when the DB lacks the goal_id column (the hasGoalId = false path). The constructor checks for this via pragma table_info, but the test schema always includes goal_id. A test with an older schema (no goal_id column) would verify graceful degradation. [fixable]
• 🔵 missing_tests: No test for listItems/getItem when the DB lacks a links table (the hasLinksTable = false path). The constructor detects this, but all tests create the full schema. A test with a schema that omits the links table would cover this backward-compatibility path. [fixable]

server/telos-store.ts

Solid integration with good test coverage for the store layer. Main concerns: fire-and-forget goal_id persistence creates a duplicate-task risk on retry, and the Telos promote route path lacks route-level test coverage.

• 🔵 unsafe_assumptions (L325): fetchAllChildren, loadSourcesForItems, and loadLinksForItems all build IN (?, ?, ...) clauses with dynamically-sized placeholder lists. SQLite has a default SQLITE_MAX_VARIABLE_NUMBER limit (typically 999-32766 depending on version). With a large item set this could hit the limit. In practice Telos likely has <100 items, but the pattern is worth noting. A chunked approach would be safer. [fixable]

[dimakis]

🟡 bugs: Fire-and-forget --set-goal via Python means if the subprocess fails (logged but swallowed), goal_id is never persisted in the Telos DB. On the next promote attempt for the same item, telosItem.goalId will still be null (readonly TelosStore re-reads from DB), bypassing the 'already promoted' guard and creating a duplicate task. The promotesInFlight Set only guards within a single in-flight request, not across sequential requests. Consider making the Python call non-fire-and-forget (await it, return 500 on failure) or at minimum logging at error level instead of warn. [fixable]

[dimakis]

🟡 regressions: The Telos read path's Zod fallback sends raw items on parse failure ({ profiles, items }) instead of empty arrays. If TelosStore returns data that doesn't match the schema (e.g., a new unexpected status value), the frontend receives unvalidated data without Zod's .default() coercions applied. The Python fallback path correctly returns { profiles: [], items: [] } on parse failure. Consider using the same empty fallback for consistency, or log the parse error to aid debugging. [fixable]

[dimakis]

🟡 missing_tests: The Telos branch of the promote endpoint has no route-level tests. The existing workload-routes.test.ts only covers the WorkloadStore branch. Missing coverage: Telos item promote (happy path), already-promoted guard (200 with existing task), deleted-task guard (409), race guard (409 in-flight), and the Python --set-goal bridge path. [fixable]

[dimakis]

🔵 style: buildPromoteDescription accepts Record<string, unknown> and both call sites cast contextHints as unknown as Record<string, unknown>. Since both TelosContextHints and the workload ContextHints are known types, the function could accept a typed union (or the shared subset) to avoid the double cast. [fixable]

[dimakis]

🔵 unsafe_assumptions: fetchAllChildren, loadSourcesForItems, and loadLinksForItems all build IN (?, ?, ...) clauses with dynamically-sized placeholder lists. SQLite has a default SQLITE_MAX_VARIABLE_NUMBER limit (typically 999-32766 depending on version). With a large item set this could hit the limit. In practice Telos likely has <100 items, but the pattern is worth noting. A chunked approach would be safer. [fixable]