fix: address PR #1641 review comments (round 3)

- Fix migration version conflict: addSessionPlatformSourceColumn now uses v25
- Sanitize observation titles in file-context deny reason (strip newlines, limit length)
- Guard json_each() with LIKE '[%' check for legacy bare-path rows
- Guard /stream SSE endpoint with 503 before DB initialization
- Scope bun-runner signal exit handling to start subcommand only
- Normalize platformSource at route boundary in DataRoutes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-04-07 14:16:41 -07:00
parent d0676aa049
commit 753a993647
8 changed files with 145 additions and 134 deletions
+1 -1
View File
@@ -196,7 +196,7 @@ child.on('close', (code, signal) => {
// Fix #1505: When the "start" subcommand forks a daemon, the parent bun
// process may be killed by signal (e.g. SIGKILL, exit code 137). The daemon
// is running fine — treat signal-based exits for "start" as success.
if (signal || (code > 128 && args.includes('start'))) {
if ((signal || code > 128) && args.includes('start')) {
process.exit(0);
}
process.exit(code || 0);
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -150,7 +150,7 @@ function formatFileTimeline(observations: ObservationRow[], filePath: string): s
const chronological = [...dayObservations].sort((a, b) => a.created_at_epoch - b.created_at_epoch);
lines.push(`### ${day}`);
for (const obs of chronological) {
const title = obs.title || 'Untitled';
const title = (obs.title || 'Untitled').replace(/[\r\n\t]/g, ' ').slice(0, 120);
const icon = TYPE_ICONS[obs.type] || '\u2753';
const time = compactTime(formatTime(obs.created_at_epoch));
lines.push(`${obs.id} ${time} ${icon} ${title}`);
+3 -3
View File
@@ -894,14 +894,14 @@ export class MigrationRunner {
}
/**
* Add platform_source column to sdk_sessions for Claude/Codex isolation (migration 24)
* Add platform_source column to sdk_sessions for Claude/Codex isolation (migration 25)
*/
private addSessionPlatformSourceColumn(): void {
const tableInfo = this.db.query('PRAGMA table_info(sdk_sessions)').all() as TableColumnInfo[];
const hasColumn = tableInfo.some(col => col.name === 'platform_source');
const indexInfo = this.db.query('PRAGMA index_list(sdk_sessions)').all() as IndexInfo[];
const hasIndex = indexInfo.some(index => index.name === 'idx_sdk_sessions_platform_source');
const applied = this.db.prepare('SELECT version FROM schema_versions WHERE version = ?').get(24) as SchemaVersion | undefined;
const applied = this.db.prepare('SELECT version FROM schema_versions WHERE version = ?').get(25) as SchemaVersion | undefined;
if (applied && hasColumn && hasIndex) return;
@@ -920,6 +920,6 @@ export class MigrationRunner {
this.db.run('CREATE INDEX IF NOT EXISTS idx_sdk_sessions_platform_source ON sdk_sessions(platform_source)');
}
this.db.prepare('INSERT OR IGNORE INTO schema_versions (version, applied_at) VALUES (?, ?)').run(24, new Date().toISOString());
this.db.prepare('INSERT OR IGNORE INTO schema_versions (version, applied_at) VALUES (?, ?)').run(25, new Date().toISOString());
}
}
+2 -2
View File
@@ -140,8 +140,8 @@ export function getObservationsByFilePath(
SELECT *
FROM observations
WHERE (
EXISTS (SELECT 1 FROM json_each(files_read) WHERE value = ?)
OR EXISTS (SELECT 1 FROM json_each(files_modified) WHERE value = ?)
(files_read LIKE '[%' AND EXISTS (SELECT 1 FROM json_each(files_read) WHERE value = ?))
OR (files_modified LIKE '[%' AND EXISTS (SELECT 1 FROM json_each(files_modified) WHERE value = ?))
)
${projectClause}
ORDER BY created_at_epoch DESC
@@ -18,6 +18,7 @@ import { SessionManager } from '../../SessionManager.js';
import { SSEBroadcaster } from '../../SSEBroadcaster.js';
import type { WorkerService } from '../../../worker-service.js';
import { BaseRouteHandler } from '../BaseRouteHandler.js';
import { normalizePlatformSource } from '../../../../shared/platform-source.js';
import { getObservationsByFilePath } from '../../../sqlite/observations/get.js';
export class DataRoutes extends BaseRouteHandler {
@@ -281,7 +282,8 @@ export class DataRoutes extends BaseRouteHandler {
*/
private handleGetProjects = this.wrapHandler((req: Request, res: Response): void => {
const store = this.dbManager.getSessionStore();
const platformSource = req.query.platformSource as string | undefined;
const rawPlatformSource = req.query.platformSource as string | undefined;
const platformSource = rawPlatformSource ? normalizePlatformSource(rawPlatformSource) : undefined;
if (platformSource) {
const projects = store.getAllProjects(platformSource);
@@ -328,7 +330,8 @@ export class DataRoutes extends BaseRouteHandler {
const offset = parseInt(req.query.offset as string, 10) || 0;
const limit = Math.min(parseInt(req.query.limit as string, 10) || 20, 100); // Max 100
const project = req.query.project as string | undefined;
const platformSource = req.query.platformSource as string | undefined;
const rawPlatformSource = req.query.platformSource as string | undefined;
const platformSource = rawPlatformSource ? normalizePlatformSource(rawPlatformSource) : undefined;
return { offset, limit, project, platformSource };
}
@@ -68,6 +68,14 @@ export class ViewerRoutes extends BaseRouteHandler {
* SSE stream endpoint
*/
private handleSSEStream = this.wrapHandler((req: Request, res: Response): void => {
// Guard: if DB is not yet initialized, return 503 before registering client
try {
this.dbManager.getSessionStore();
} catch {
res.status(503).json({ error: 'Service initializing' });
return;
}
// Setup SSE headers
res.setHeader('Content-Type', 'text/event-stream');
res.setHeader('Cache-Control', 'no-cache');