Handoff merged master work and UI refinements#39
Conversation
Evaluate trusted AI review scorecards in CI so pull requests can be blocked, flagged for fixes, or marked auto-approve eligible without weakening branch protection.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Use a trusted caller-driven gate workflow, preserve raw scores at threshold boundaries, and tighten GitHub output and approval handling around the PR review feedback.
Replace error part counting with message-timestamp-based staleness. Errors now expire after ERROR_STALE_MS (60s) based on actual message creation time. Adds session-diff utility for status change tracking. Updates test mocks to match new ErrorMessageRow/LatestMessageRow shape.
Replace sequential source processing with parallel git operations via Promise.all. Pre-filter sessions using findIncludedSessionsSqlite before expensive getMainSessionViewSqlite calls. Add idleTimeout: 60 to prevent Bun.serve from killing long cold-load requests. Reduces /api/projects cold load from ~30s to ~9s.
…redesign Detect running_script via SCRIPT_TOOL_NAMES in computeDisplayStatus. Add strip-tool-badge component showing current tool name. Redesign worktree badge to narrow stacked X/divider/Y format. Add --status-script and --status-script-glow design tokens. Add WorktreeInfo/WorktreeSummary types to ProjectSnapshot.
Replace inline STATUS_PRIORITY with ATTENTION_FIRST_PRIORITY from status-rollup. Migrate overlay state to ActiveOverlay union type. Add computeProjectSoundDecisions for per-session status tracking. Update expanded card styling and minor CSS adjustments.
…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.
…ling and source cache invalidation
There was a problem hiding this comment.
Pull request overview
This PR merges remaining handoff-branch work focused on improving session/status handling (including SQLite derivations and sound notification diffs), adds optional Telegram status/alert notifications on the server, and refines multiple UI components (project strip visuals, density sizing, and timeout labeling).
Changes:
- Add a Telegram notification service (pinned status message + alert messages) and expose
/api/telegram/status. - Refactor status/sound transition detection via a session-diff model; improve SQLite-backed status derivation and DB reuse.
- UI refinements: add
running_scriptdisplay status + tool badge, adjust density sizing/gaps, and tweak question visuals/labels.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/components/Sparkline.css | Update “sand” sparkline glow colors. |
| src/ui/components/SettingsPanel.tsx | Show sub-minute timeout labels in seconds; adjust slider min/ARIA label. |
| src/ui/components/SessionSwimlane.css | Update “sand” legend dot color. |
| src/ui/components/ProjectStrip.tsx | Add running_script display status + tool badge; tweak stale logic and worktree badge markup. |
| src/ui/components/ProjectStrip.css | Add styles for running_script, tool badge, updated question dot, and density sizing tweaks. |
| src/ui/components/ProjectManagementPanel.css | Prevent grid items from overflowing via min-width: 0. |
| src/ui/components/ProjectCard.css | Switch to grid layout and add responsive action layout improvements. |
| src/ui/components/DashboardHeader.tsx | Rename displayed dashboard title. |
| src/ui/App.tsx | Refactor sound playback decisions via session diffs; adjust ordering/overlay state handling. |
| src/ui/App.css | Make project stack gap respect --grid-gap variable. |
| src/types.ts | Tighten/extend types (plan/boulder status unions, worktrees, Telegram types, ordering state). |
| src/styles/tokens.css | Add script status color tokens. |
| src/server/telegram.ts | New Telegram polling + pinned status message + alert notifications service. |
| src/server/start.ts | Wire in multi-project service + optional Telegram service in production start. |
| src/server/dev.ts | Wire in multi-project service + optional Telegram service in dev server. |
| src/server/multi-project.ts | Reuse shared SQLite DB per poll; cache session summaries; parallelize git ops. |
| src/server/dashboard.ts | Reuse a single SQLite DB handle per payload build; improve fallback behavior. |
| src/server/api.ts | Inject multi-project service; add /telegram/status; adjust source routes. |
| src/ingest/storage-backend.ts | Allow passing a pre-opened readonly SQLite DB into read helpers. |
| src/ingest/sqlite-derive.ts | Thread optional DB handle through derive paths; refine terminal error detection; add multi-session helpers. |
| src/ingest/session.ts | Refine file-based terminal error detection with staleness threshold; improve question bridging. |
| src/ingest/session-inclusion.ts | Use JSON extraction from SQLite data columns; add terminal error staleness handling; cache derived statuses. |
| src/ingest/session-diff.ts | New module to diff session statuses and decide sound playback. |
| src/ingest/activity-status.ts | Add ERROR_STALE_MS constant and a helper for terminal error timestamps. |
| src/tests/session-inclusion.test.ts | Update mocks/tests for new terminal status query logic and staleness semantics. |
| src/tests/question-bridge.test.ts | Stabilize module mocking and add SQLite question-bridge coverage. |
| src/tests/api.test.ts | Update API tests to inject multiProjectService directly. |
| README.md | Add Ko-fi badge. |
| .env.example | Document Telegram env vars. |
[RISK:medium]
[SCORES]
{"security": 3.5, "safety": 3.0, "performance": 4.0, "featureQuality": 2.5, "confidence": 4.0}
[/SCORES]
[SUMMARY]
Key correctness issues remain around project ordering being unintentionally “manualized,” and API input/schema consistency for the new Telegram/status and source label update route. A couple of focused changes plus added unit coverage for the new diff/sound and running_script logic would significantly reduce regression risk.
[/SUMMARY]
| function updatePrevStatuses(payload: DashboardMultiProjectPayload): void { | ||
| state.prevSessionStatuses.clear() | ||
| for (const project of payload.projects) { | ||
| for (const session of project.sessions) { | ||
| state.prevSessionStatuses.set(session.sessionId, session.status) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[SEVERITY:nit] [DIM:featureQuality]
state.prevSessionStatuses is written to and cleared, but never read. Either remove it from TelegramState/updatePrevStatuses or use it for transition-based alerting; keeping unused state makes the service harder to reason about.
|
|
||
| cachedPayload = payload | ||
| cachedPayloadAt = nowMs | ||
| cachedPayloadAt = Date.now() |
There was a problem hiding this comment.
[SEVERITY:nit] [DIM:featureQuality]
cachedPayloadAt is updated with a fresh Date.now() even though nowMs was already captured at the top of the function and is what TTL calculations use. Assign cachedPayloadAt = nowMs for consistency and to avoid tiny clock-skew edge cases.
| cachedPayloadAt = Date.now() | |
| cachedPayloadAt = nowMs |
| // --------------------------------------------------------------------------- | ||
| api.put("/sources/:sourceId", async (c) => { | ||
| const sourceId = c.req.param("sourceId") | ||
| const body = await c.req.json<{ label?: string }>() |
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:safety]
The PUT /sources/:sourceId route no longer validates that label is a string. If a client sends a non-string value, it will be persisted into sources.json and can break consumers that assume label?: string. Reintroduce input validation (400 on invalid type) before calling updateSourceLabelById.
| const body = await c.req.json<{ label?: string }>() | |
| const body = await c.req.json<{ label?: string }>() | |
| if (body.label !== undefined && typeof body.label !== "string") { | |
| return c.json({ ok: false, error: "Invalid label" }, 400) | |
| } |
| if (aggregateStatus === 'running_tool' && currentTool && SCRIPT_TOOL_NAMES.has(currentTool)) { | ||
| return 'running_script' | ||
| } |
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:featureQuality]
computeDisplayStatus can now return running_script based on currentTool, and getSessionFamily treats it as active. The existing project-strip-status tests cover computeDisplayStatus/getSessionFamily, but there are no assertions for the new running_script branch. Add/update tests to cover script-tool detection and the new family mapping.
| export function shouldPlaySound(diff: SessionStatusDiff, config: SoundConfig): SoundPlaybackDecision { | ||
| if (!config.enabled) { | ||
| return { | ||
| playWaiting: false, | ||
| playAllClear: false, | ||
| playAttention: false, | ||
| playQuestion: false, | ||
| } | ||
| } | ||
|
|
||
| const playQuestion = config.onQuestion | ||
| ? hasStatus(diff.newSessions, "question") || hasStatus(diff.changedSessions, "question") | ||
| : false | ||
|
|
||
| const playAttention = config.onSessionError | ||
| ? hasStatus(diff.newSessions, "error") || hasStatus(diff.changedSessions, "error") | ||
| : false | ||
|
|
||
| const playWaiting = config.onSessionIdle ? hasIdleFromActive(diff.changedSessions) : false | ||
|
|
||
| const playAllClear = config.onPlanComplete ? diff.planCompleted : false | ||
|
|
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:featureQuality]
diffSessionStatuses/shouldPlaySound introduce new core behavior for sound notifications (new sessions, removals, status transitions, plan completion). Add unit tests to lock down expected diffs and playback decisions across common transitions (idle-from-active, error/question transitions, plan complete) so regressions are caught.
| 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:info] [DIM:api_design]
GET /telegram/status returns different shapes depending on whether Telegram is configured ({ enabled: false } vs full TelegramServiceStatus). This makes the API response harder to consume safely and doesn’t match the TelegramServiceStatus type added in src/types.ts. Consider always returning the full shape (with null/0 defaults) so clients can rely on a stable schema.
| function formatTimeout(ms: number): string { | ||
| if (ms < 60_000) return `${Math.round(ms / 1_000)}s` | ||
| const minutes = Math.round(ms / 60_000) | ||
| if (minutes < 1) return "<1 min" |
There was a problem hiding this comment.
[SEVERITY:nit] [DIM:featureQuality]
formatTimeout now returns seconds for values under 60s, which makes the if (minutes < 1) return "<1 min" branch unreachable because minutes is only computed when ms >= 60_000. Remove the dead branch (or adjust the logic) to avoid misleading/unused code paths.
| if (minutes < 1) return "<1 min" |
| () => resolveProjectOrderIds(sortedProjects, orderedIds, orderedIds.length > 0), | ||
| [sortedProjects, orderedIds], |
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:featureQuality]
Project ordering is effectively forced into “manual order” as soon as syncIds runs (because orderedIds becomes non-empty), so status/recency sorting stops applying on subsequent updates/reloads. Since resolveProjectOrderIds already accepts an isManualOrder flag (and ProjectOrderState now includes it), use that flag here instead of orderedIds.length > 0, and ensure DnD reordering is what flips isManualOrder to true.
| () => resolveProjectOrderIds(sortedProjects, orderedIds, orderedIds.length > 0), | |
| [sortedProjects, orderedIds], | |
| () => resolveProjectOrderIds(sortedProjects, orderedIds, isManualOrder), | |
| [sortedProjects, orderedIds, isManualOrder], |
| columns: number | ||
| isManualOrder: boolean | ||
| } | ||
|
|
There was a problem hiding this comment.
[SEVERITY:warning] [DIM:safety]
ProjectOrderState now requires isManualOrder, but persisted/default states (e.g., older localStorage values) won’t have this field. Add a backwards-compatible default/migration (e.g., default isManualOrder to false when missing) so existing users don’t end up with undefined behavior and so state parsing remains robust.
| /** Backwards-compatible persisted project ordering state */ | |
| export type PersistedProjectOrderState = Omit<ProjectOrderState, "isManualOrder"> & { | |
| isManualOrder?: boolean | |
| } | |
| /** Normalize persisted project ordering state, defaulting legacy values safely */ | |
| export function normalizeProjectOrderState(state: PersistedProjectOrderState): ProjectOrderState { | |
| return { | |
| ...state, | |
| isManualOrder: state.isManualOrder ?? false, | |
| } | |
| } |
Summary
deploy-masterand dirtyfix-nand-ago-bug) to avoid regressions or losing uncommitted workVerification
bun x vitest run src/__tests__(201 passed)bun run build