fix: add missing migration 28 mirror in SessionStore (#2139) (#2140)

* fix: mirror migration 28 in SessionStore so pending_messages.tool_use_id and worker_pid columns are created (#2139)

SessionStore's inline migration list jumped from v27 to v29, skipping
rebuildPendingMessagesForSelfHealingClaim. The worker uses SessionStore
directly via worker/DatabaseManager.ts and bypasses the canonical
MigrationRunner, so fresh installs ended up at "max v29" with neither
column present — every queue claim and observation insert failed.

Adds addPendingMessagesToolUseIdAndWorkerPidColumns following the existing
mirror precedent (addObservationSubagentColumns / addObservationsUniqueContentHashIndex).
Uses ALTER TABLE + column-existence guards so already-broken DBs at v29
self-heal on next worker boot.

Verified on fresh DB and on a synthetic v29-without-v28 broken DB:
both columns and indexes (idx_pending_messages_worker_pid,
ux_pending_session_tool) appear after one boot.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: wrap v28 mirror dedup+index creation in transaction

Addresses Greptile P2 review on PR #2140: matches the existing pattern in
addObservationsUniqueContentHashIndex (v29 mirror at SessionStore.ts:1127)
and runner.ts rebuildPendingMessagesForSelfHealingClaim. A crash between
the dedup DELETE and the schema_versions INSERT no longer leaves the DB
in a half-applied state.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-04-25 18:45:51 -07:00
committed by GitHub
parent 3e01b62f72
commit e8082bb992
3 changed files with 105 additions and 4 deletions
+77
View File
@@ -73,6 +73,7 @@ export class SessionStore {
this.addObservationModelColumns();
this.ensureMergedIntoProjectColumns();
this.addObservationSubagentColumns();
this.addPendingMessagesToolUseIdAndWorkerPidColumns();
this.addObservationsUniqueContentHashIndex();
}
@@ -1038,6 +1039,82 @@ export class SessionStore {
}
}
/**
* Add tool_use_id and worker_pid columns + indexes to pending_messages (migration 28).
*
* Mirrors MigrationRunner.rebuildPendingMessagesForSelfHealingClaim so bundled
* artifacts that embed SessionStore (e.g. worker-service.cjs, context-generator.cjs)
* stay schema-consistent. Without this, every queue-claim cycle fails with
* "no such column: worker_pid" and every observation insert fails with
* "table pending_messages has no column named tool_use_id" (issue #2139).
*
* Uses ALTER TABLE rather than the full table rebuild from MigrationRunner because:
* - It's safe on populated DBs that already reached v29 without ever applying v28.
* - The legacy stale-reset epoch column the rebuild dropped never existed in
* pending_messages tables created by the SessionStore migration path.
*
* Column existence is checked directly — schema_versions cannot be trusted because
* affected DBs may already have v29 recorded with neither column present (#2139).
*/
private addPendingMessagesToolUseIdAndWorkerPidColumns(): void {
// pending_messages may not exist yet on freshly-created DBs at this point in
// the migration order — createPendingMessagesTable (v16) has already run by
// the time we get here, so this guard is defensive only.
const tables = this.db.query(
"SELECT name FROM sqlite_master WHERE type='table' AND name='pending_messages'"
).all() as TableNameRow[];
if (tables.length === 0) {
this.db.prepare('INSERT OR IGNORE INTO schema_versions (version, applied_at) VALUES (?, ?)').run(28, new Date().toISOString());
return;
}
const cols = this.db.query('PRAGMA table_info(pending_messages)').all() as TableColumnInfo[];
const hasToolUseId = cols.some(c => c.name === 'tool_use_id');
const hasWorkerPid = cols.some(c => c.name === 'worker_pid');
if (!hasToolUseId) {
this.db.run('ALTER TABLE pending_messages ADD COLUMN tool_use_id TEXT');
}
if (!hasWorkerPid) {
this.db.run('ALTER TABLE pending_messages ADD COLUMN worker_pid INTEGER');
}
// Wrap dedup DELETE + UNIQUE index creation + version-record in a transaction
// so a crash mid-flight cannot leave duplicates removed without v28 recorded.
// Matches addObservationsUniqueContentHashIndex (v29) at line 1127 and
// runner.ts rebuildPendingMessagesForSelfHealingClaim (v28).
this.db.run('BEGIN TRANSACTION');
try {
// Indexes are idempotent — match runner.ts:1117-1120 + 1134-1138.
this.db.run('CREATE INDEX IF NOT EXISTS idx_pending_messages_worker_pid ON pending_messages(worker_pid)');
// The UNIQUE partial index requires no duplicate (content_session_id, tool_use_id)
// pairs. Dedup before creating it (matches runner.ts:1124-1132). Safe to run
// unconditionally — if tool_use_id was just added, every row has it as NULL
// and the WHERE filter excludes them.
this.db.run(`
DELETE FROM pending_messages
WHERE tool_use_id IS NOT NULL
AND id NOT IN (
SELECT MIN(id) FROM pending_messages
WHERE tool_use_id IS NOT NULL
GROUP BY content_session_id, tool_use_id
)
`);
this.db.run(`
CREATE UNIQUE INDEX IF NOT EXISTS ux_pending_session_tool
ON pending_messages(content_session_id, tool_use_id)
WHERE tool_use_id IS NOT NULL
`);
this.db.prepare('INSERT OR IGNORE INTO schema_versions (version, applied_at) VALUES (?, ?)').run(28, new Date().toISOString());
this.db.run('COMMIT');
} catch (error) {
this.db.run('ROLLBACK');
throw error;
}
}
/**
* Add UNIQUE(memory_session_id, content_hash) on observations (migration 29).
* Mirrors MigrationRunner.addObservationsUniqueContentHashIndex so bundled