feat: improve dashboard aggregation and telegram support#38
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves multi-project dashboard aggregation efficiency (SQLite reuse + caching) and adds optional Telegram notification support (startup wiring + status endpoint), alongside minor UI/docs tweaks.
Changes:
- Rework multi-project aggregation to reuse a shared readonly SQLite connection and cache per-project session summaries/time series.
- Add a Telegram notification service (pinned status message + alerts) and expose its status via a new API endpoint.
- Minor UI styling adjustment for “question” status and documentation/environment updates.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/components/ProjectStrip.css | Refines “question” status dot styling. |
| src/types.ts | Adds Telegram config/status types. |
| src/server/telegram.ts | Introduces Telegram polling/alerting service implementation. |
| src/server/start.ts | Wires multi-project service + optional Telegram service into prod server startup and API. |
| src/server/dev.ts | Same wiring as prod for dev server; adds idleTimeout. |
| src/server/multi-project.ts | Uses shared SQLite DB + caching to reduce redundant reads; parallelizes git ops. |
| src/server/dashboard.ts | Reuses a single SQLite DB handle across multiple derive calls; closes reliably. |
| src/server/api.ts | Accepts injected multi-project service; adds GET /telegram/status. |
| src/ingest/storage-backend.ts | Adds withDbOrOpen and threads optional pre-opened DB through SQLite readers. |
| src/ingest/sqlite-derive.ts | Threads optional DB through derive helpers; adds multi-session helper functions. |
| src/ingest/session-inclusion.ts | Caches derived status during inclusion/sorting to avoid repeated derivations. |
| src/tests/api.test.ts | Updates API tests to inject a mock multi-project service. |
| README.md | Adds Ko-fi badge link. |
| .env.example | Documents Telegram env vars. |
[RISK:medium]
[SCORES]
{"security":4,"safety":3,"performance":4,"featureQuality":3,"confidence":4}
[/SCORES]
[SUMMARY]
Main concerns are the Telegram polling loop potentially running overlapping async polls (state races / extra API calls) and missing test coverage for the new /telegram/status endpoint, plus an inconsistent response shape when Telegram is disabled.
[/SUMMARY]
| function start(): void { | ||
| if (timer !== null) return | ||
| poll() | ||
| timer = setInterval(poll, pollIntervalMs) | ||
| } |
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:safety]
start() schedules poll() via setInterval(poll, pollIntervalMs) even though poll() is async. If a poll takes longer than the interval, multiple polls can overlap and race on shared state (and potentially trigger extra Telegram API calls / rate limiting). Add an in-flight guard (e.g., a boolean mutex) or switch to a self-scheduling setTimeout loop that awaits poll() completion before scheduling the next run.
|
|
||
| type TelegramState = { | ||
| pinnedMessageId: number | null | ||
| prevSessionStatuses: Map<string, SessionStatus> |
There was a problem hiding this comment.
[SEVERITY:nit] [DIM:featureQuality]
prevSessionStatuses is part of TelegramState but isn’t read anywhere in this module, so it’s dead state. Consider removing it (and the related updater) or using it to drive alerting based on actual status transitions.
| prevSessionStatuses: Map<string, SessionStatus> |
| api.get("/telegram/status", (c) => { | ||
| if (!opts.telegramStatus) { | ||
| return c.json({ ok: true, telegram: { enabled: false } }) | ||
| } |
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:featureQuality]
When Telegram is disabled, this endpoint returns telegram: { enabled: false }, but when enabled it returns a full TelegramServiceStatus. Returning different shapes makes clients harder to type and handle. Consider always returning a full status object (with pinnedMessageId/lastUpdateMs/lastError as null and alertsSent as 0) even when disabled.
| api.get("/telegram/status", (c) => { | ||
| if (!opts.telegramStatus) { | ||
| return c.json({ ok: true, telegram: { enabled: false } }) | ||
| } | ||
| return c.json({ ok: true, telegram: opts.telegramStatus() }) | ||
| }) |
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:featureQuality]
A new API route (GET /telegram/status) was added, but the API route test suite in src/__tests__/api.test.ts doesn’t cover it. Add tests for both cases (telegramStatus unset → disabled response, and telegramStatus provided → returns status) to keep route coverage consistent.
…f any-error-in-history Error status was triggered by any tool part with status 'error' in recent history, causing recoverable refusals (e.g. 'read file before editing') to persist as Error state. Now only the most recent terminal tool event (error or completed) determines error status. Also fixes session-inclusion.ts SQL queries that referenced non-existent columns (state_status, tool) instead of using json_extract on the data JSON column.
Summary