mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-10 09:15:24 +00:00
fix(core): salvage session usage into the history before deleting transcripts (#7391)
usageHistoryService's rebuild fallback derives per-session usage summaries from session transcript JSONLs, and persistSessionUsage only runs on /clear and process exit. removeSessionFiles() deleted the transcript without either — so deleting a session that was never /clear'ed or cleanly exited permanently erased its tokens from the usage records (#7384). removeSessionFiles() now calls a new persistUsageBeforeTranscriptDeletion() on the transcript (active or archived branch) right before unlinking it. The salvage replays the transcript's ui_telemetry records through the same summarization the rebuild migration uses — extracted into a shared summarizeTranscript() so the two cannot drift — skips the write when usage_record.jsonl already carries the session (a /clear or exit wrote the authoritative record; duplicating would re-open #4994), and never throws, so deletion always proceeds. Verified against the real compiled services under a temp QWEN_HOME: before, removeSession() leaves usage_record.jsonl without the session forever; after, the summary (with its token totals) survives deletion. The wiring test pins the salvage running BEFORE unlink and fails on the unpatched source. Fixes #7384 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
2803f82c09
commit
4af784d2bf
4 changed files with 240 additions and 39 deletions
|
|
@ -5,6 +5,7 @@
|
|||
*/
|
||||
|
||||
import fs from 'node:fs';
|
||||
import { persistUsageBeforeTranscriptDeletion } from './usageHistoryService.js';
|
||||
import path from 'node:path';
|
||||
import { Readable } from 'node:stream';
|
||||
import {
|
||||
|
|
@ -38,6 +39,9 @@ import { CompressionStatus } from '../core/turn.js';
|
|||
import type { ChatRecord } from './chatRecordingService.js';
|
||||
import * as jsonl from '../utils/jsonl-utils.js';
|
||||
|
||||
vi.mock('./usageHistoryService.js', () => ({
|
||||
persistUsageBeforeTranscriptDeletion: vi.fn().mockResolvedValue(true),
|
||||
}));
|
||||
vi.mock('node:path');
|
||||
vi.mock('../utils/paths.js');
|
||||
vi.mock('../utils/runtimeStatus.js');
|
||||
|
|
@ -1439,6 +1443,15 @@ describe('SessionService', () => {
|
|||
|
||||
expect(result).toBe(true);
|
||||
expect(unlinkSyncSpy).toHaveBeenCalled();
|
||||
// #7384: the usage salvage must see the transcript BEFORE it is
|
||||
// unlinked, or the summary is unrecoverable.
|
||||
const salvage = vi.mocked(persistUsageBeforeTranscriptDeletion);
|
||||
expect(salvage).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`${sessionIdA}.jsonl`),
|
||||
);
|
||||
expect(salvage.mock.invocationCallOrder[0]!).toBeLessThan(
|
||||
unlinkSyncSpy.mock.invocationCallOrder[0]!,
|
||||
);
|
||||
expect(rmSyncSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`file-history/${sessionIdA}`),
|
||||
{ recursive: true, force: true },
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
*/
|
||||
|
||||
import { Storage } from '../config/storage.js';
|
||||
import { persistUsageBeforeTranscriptDeletion } from './usageHistoryService.js';
|
||||
import { getProjectHash } from '../utils/paths.js';
|
||||
import path from 'node:path';
|
||||
import fs from 'node:fs';
|
||||
|
|
@ -1341,6 +1342,10 @@ export class SessionService {
|
|||
const activePath = this.getSessionFilePath(sessionId, 'active');
|
||||
const active = await this.readProjectSessionHead(sessionId, activePath);
|
||||
if (active) {
|
||||
// #7384: the usage-history rebuild reads transcripts, so salvage
|
||||
// the session's usage summary before the file is gone. Never
|
||||
// blocks deletion (the salvage swallows its own errors).
|
||||
await persistUsageBeforeTranscriptDeletion(activePath);
|
||||
this.removeFileIfExists(activePath);
|
||||
const archivedPath = this.getSessionFilePath(sessionId, 'archived');
|
||||
if (fs.existsSync(archivedPath)) {
|
||||
|
|
@ -1358,6 +1363,7 @@ export class SessionService {
|
|||
if (!archived) {
|
||||
return false;
|
||||
}
|
||||
await persistUsageBeforeTranscriptDeletion(archivedPath);
|
||||
this.removeFileIfExists(archivedPath);
|
||||
this.removeWorktreeSidecars(sessionId);
|
||||
this.removeFileHistoryBackups(sessionId);
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import {
|
|||
loadUsageHistory,
|
||||
loadUsageHistoryWithLive,
|
||||
persistSessionUsage,
|
||||
persistUsageBeforeTranscriptDeletion,
|
||||
} from './usageHistoryService.js';
|
||||
import { ToolCallDecision } from '../telemetry/tool-call-decision.js';
|
||||
import type { SessionMetrics } from '../telemetry/uiTelemetry.js';
|
||||
|
|
@ -777,6 +778,128 @@ describe('loadUsageHistory + persistSessionUsage (issue #4994 regression)', () =
|
|||
});
|
||||
});
|
||||
|
||||
// Regression for #7384: deleting a session erased its usage from the
|
||||
// rebuild-from-transcript fallback forever. The salvage runs right before
|
||||
// transcript deletion.
|
||||
describe('persistUsageBeforeTranscriptDeletion (issue #7384)', () => {
|
||||
let tmpHome: string;
|
||||
let originalQwenHome: string | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-usage-salvage-'));
|
||||
originalQwenHome = process.env['QWEN_HOME'];
|
||||
process.env['QWEN_HOME'] = path.join(tmpHome, '.qwen');
|
||||
fs.mkdirSync(process.env['QWEN_HOME'], { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalQwenHome === undefined) delete process.env['QWEN_HOME'];
|
||||
else process.env['QWEN_HOME'] = originalQwenHome;
|
||||
fs.rmSync(tmpHome, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function plantTranscript(sessionId: string, withTelemetry: boolean): string {
|
||||
const dir = path.join(
|
||||
process.env['QWEN_HOME']!,
|
||||
'projects',
|
||||
'salvage-project',
|
||||
'chats',
|
||||
);
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
const filePath = path.join(dir, `${sessionId}.jsonl`);
|
||||
const start = new Date('2026-07-01T00:00:00Z').toISOString();
|
||||
const mid = new Date('2026-07-01T00:01:00Z').toISOString();
|
||||
const records: unknown[] = [
|
||||
{
|
||||
sessionId,
|
||||
cwd: '/salvage/project',
|
||||
uuid: 'u1',
|
||||
parentUuid: null,
|
||||
timestamp: start,
|
||||
type: 'user',
|
||||
message: { role: 'user', content: 'hi' },
|
||||
},
|
||||
];
|
||||
if (withTelemetry) {
|
||||
records.push({
|
||||
sessionId,
|
||||
cwd: '/salvage/project',
|
||||
uuid: 'u2',
|
||||
parentUuid: 'u1',
|
||||
timestamp: mid,
|
||||
type: 'system',
|
||||
subtype: 'ui_telemetry',
|
||||
systemPayload: {
|
||||
uiEvent: {
|
||||
'event.name': 'qwen-code.api_response',
|
||||
'event.timestamp': mid,
|
||||
response_id: 'r1',
|
||||
model: 'qwen-max',
|
||||
duration_ms: 900,
|
||||
input_token_count: 600,
|
||||
output_token_count: 300,
|
||||
cached_content_token_count: 0,
|
||||
thoughts_token_count: 100,
|
||||
total_token_count: 1000,
|
||||
prompt_id: 'p1',
|
||||
},
|
||||
},
|
||||
});
|
||||
}
|
||||
fs.writeFileSync(
|
||||
filePath,
|
||||
records.map((r) => JSON.stringify(r)).join('\n') + '\n',
|
||||
);
|
||||
return filePath;
|
||||
}
|
||||
|
||||
function usagePath(): string {
|
||||
return path.join(process.env['QWEN_HOME']!, 'usage_record.jsonl');
|
||||
}
|
||||
|
||||
it('writes the session summary before the transcript disappears', async () => {
|
||||
const filePath = plantTranscript('sess-salvage-1', true);
|
||||
await expect(persistUsageBeforeTranscriptDeletion(filePath)).resolves.toBe(
|
||||
true,
|
||||
);
|
||||
const lines = fs
|
||||
.readFileSync(usagePath(), 'utf-8')
|
||||
.trim()
|
||||
.split('\n')
|
||||
.map((l) => JSON.parse(l));
|
||||
expect(lines).toHaveLength(1);
|
||||
expect(lines[0].sessionId).toBe('sess-salvage-1');
|
||||
expect(lines[0].models['qwen-max'].totalTokens).toBe(1000);
|
||||
expect(lines[0].project).toBe('/salvage/project');
|
||||
});
|
||||
|
||||
it('skips the write when the history already has the session (no #4994 duplicates)', async () => {
|
||||
const filePath = plantTranscript('sess-salvage-2', true);
|
||||
await persistUsageBeforeTranscriptDeletion(filePath);
|
||||
await expect(persistUsageBeforeTranscriptDeletion(filePath)).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
const lines = fs.readFileSync(usagePath(), 'utf-8').trim().split('\n');
|
||||
expect(lines).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('returns false for a transcript with no telemetry and writes nothing', async () => {
|
||||
const filePath = plantTranscript('sess-salvage-3', false);
|
||||
await expect(persistUsageBeforeTranscriptDeletion(filePath)).resolves.toBe(
|
||||
false,
|
||||
);
|
||||
expect(fs.existsSync(usagePath())).toBe(false);
|
||||
});
|
||||
|
||||
it('never throws for a missing transcript', async () => {
|
||||
await expect(
|
||||
persistUsageBeforeTranscriptDeletion(
|
||||
path.join(tmpHome, 'nope', 'missing.jsonl'),
|
||||
),
|
||||
).resolves.toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('aggregateUsage — skills', () => {
|
||||
function skillRecord(
|
||||
sessionId: string,
|
||||
|
|
|
|||
|
|
@ -222,6 +222,98 @@ export function metricsToUsageRecord(
|
|||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Replays a session transcript's `ui_telemetry` records into a usage
|
||||
* summary. Returns null for an empty transcript; `record` is null when the
|
||||
* transcript has no telemetry events (or unparseable timestamps) — the
|
||||
* sessionId is still reported so callers can dedupe by session.
|
||||
*
|
||||
* Single implementation shared by the rebuild migration and the
|
||||
* pre-deletion salvage (#7384) so the two can never drift.
|
||||
*/
|
||||
function summarizeTranscript(
|
||||
records: ChatRecord[],
|
||||
): { sessionId: string; record: UsageSummaryRecord | null } | null {
|
||||
if (records.length === 0) return null;
|
||||
const firstRecord = records[0]!;
|
||||
const sessionId = firstRecord.sessionId;
|
||||
if (!sessionId) return null;
|
||||
const project = firstRecord.cwd;
|
||||
|
||||
const telemetry = new UiTelemetryService();
|
||||
let hasEvents = false;
|
||||
for (const record of records) {
|
||||
if (record.type === 'system' && record.subtype === 'ui_telemetry') {
|
||||
const payload = record.systemPayload as { uiEvent?: UiEvent } | undefined;
|
||||
if (payload?.uiEvent) {
|
||||
telemetry.addEvent(payload.uiEvent);
|
||||
hasEvents = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!hasEvents) return { sessionId, record: null };
|
||||
|
||||
const startTime = new Date(firstRecord.timestamp).getTime();
|
||||
const endTime = new Date(records[records.length - 1]!.timestamp).getTime();
|
||||
if (isNaN(startTime) || isNaN(endTime)) {
|
||||
return { sessionId, record: null };
|
||||
}
|
||||
return {
|
||||
sessionId,
|
||||
record: metricsToUsageRecord(
|
||||
sessionId,
|
||||
project,
|
||||
startTime,
|
||||
endTime,
|
||||
telemetry.getMetrics(),
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Salvages a session's usage summary into `usage_record.jsonl` right before
|
||||
* its transcript is deleted (#7384): the usage-history rebuild reads
|
||||
* transcripts, so deleting a session that was never `/clear`ed or cleanly
|
||||
* exited previously erased its usage from the records forever.
|
||||
*
|
||||
* Never throws — deletion must proceed even when salvage fails — and skips
|
||||
* the write when the persisted history already carries a record for the
|
||||
* session (a `/clear` or exit already wrote the authoritative summary;
|
||||
* duplicating it would re-open #4994). Returns true when a record was
|
||||
* written.
|
||||
*/
|
||||
export async function persistUsageBeforeTranscriptDeletion(
|
||||
transcriptPath: string,
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
const records = await jsonl.read<ChatRecord>(transcriptPath);
|
||||
const summarized = summarizeTranscript(records);
|
||||
if (!summarized?.record) return false;
|
||||
|
||||
const usagePath = getUsageHistoryPath();
|
||||
try {
|
||||
if (fs.existsSync(usagePath)) {
|
||||
const existing = await jsonl.read<UsageSummaryRecord>(usagePath);
|
||||
if (existing.some((r) => r?.sessionId === summarized.sessionId)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
} catch (e) {
|
||||
// Unreadable history: write anyway — the read side dedupes by
|
||||
// sessionId (last-wins), so a duplicate is bounded and preferable to
|
||||
// silently losing the session's usage.
|
||||
debugLogger.debug(
|
||||
`persistUsageBeforeTranscriptDeletion: cannot read history: ${e}`,
|
||||
);
|
||||
}
|
||||
jsonl.writeLineSync(usagePath, summarized.record);
|
||||
return true;
|
||||
} catch (e) {
|
||||
debugLogger.debug(`persistUsageBeforeTranscriptDeletion: ${e}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
interface RebuildFromSessionJsonlOptions {
|
||||
/**
|
||||
* Session to exclude from the one-time persistence migration (the caller's
|
||||
|
|
@ -315,45 +407,12 @@ async function rebuildFromSessionJsonl(
|
|||
}
|
||||
|
||||
const records = await jsonl.read<ChatRecord>(filePath);
|
||||
if (records.length === 0) continue;
|
||||
|
||||
const firstRecord = records[0]!;
|
||||
const sessionId = firstRecord.sessionId;
|
||||
if (seenSessionIds.has(sessionId)) continue;
|
||||
seenSessionIds.add(sessionId);
|
||||
const project = firstRecord.cwd;
|
||||
|
||||
const telemetry = new UiTelemetryService();
|
||||
let hasEvents = false;
|
||||
|
||||
for (const record of records) {
|
||||
if (record.type === 'system' && record.subtype === 'ui_telemetry') {
|
||||
const payload = record.systemPayload as
|
||||
| { uiEvent?: UiEvent }
|
||||
| undefined;
|
||||
if (payload?.uiEvent) {
|
||||
telemetry.addEvent(payload.uiEvent);
|
||||
hasEvents = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!hasEvents) continue;
|
||||
|
||||
const startTime = new Date(firstRecord.timestamp).getTime();
|
||||
const lastRecord = records[records.length - 1]!;
|
||||
const endTime = new Date(lastRecord.timestamp).getTime();
|
||||
if (isNaN(startTime) || isNaN(endTime) || !sessionId) continue;
|
||||
|
||||
results.push(
|
||||
metricsToUsageRecord(
|
||||
sessionId,
|
||||
project,
|
||||
startTime,
|
||||
endTime,
|
||||
telemetry.getMetrics(),
|
||||
),
|
||||
);
|
||||
const summarized = summarizeTranscript(records);
|
||||
if (!summarized) continue;
|
||||
if (seenSessionIds.has(summarized.sessionId)) continue;
|
||||
seenSessionIds.add(summarized.sessionId);
|
||||
if (!summarized.record) continue;
|
||||
results.push(summarized.record);
|
||||
} catch (e) {
|
||||
debugLogger.debug(
|
||||
`rebuildFromSessionJsonl: failed to process ${file}: ${e}`,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue