mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(daemon): harden artifact restore metadata
This commit is contained in:
parent
42c19ccfa3
commit
07e5956418
5 changed files with 149 additions and 8 deletions
|
|
@ -2056,16 +2056,27 @@ describe('SessionArtifactStore', () => {
|
|||
});
|
||||
|
||||
it('marks a stored workspace artifact missing if it later escapes by symlink', async () => {
|
||||
const persistence = {
|
||||
recordEvent: vi.fn(),
|
||||
recordSnapshot: vi.fn(),
|
||||
};
|
||||
const store = new SessionArtifactStore({
|
||||
sessionId: 's7-symlink-refresh',
|
||||
workspaceCwd: workspace,
|
||||
persistence,
|
||||
});
|
||||
const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-outside-'));
|
||||
try {
|
||||
await fs.writeFile(path.join(workspace, 'report.txt'), 'hello');
|
||||
await fs.writeFile(path.join(outside, 'secret.txt'), 'secret');
|
||||
await store.upsertMany(
|
||||
[{ title: 'Report', workspacePath: 'report.txt' }],
|
||||
[
|
||||
{
|
||||
title: 'Report',
|
||||
workspacePath: 'report.txt',
|
||||
retention: 'restorable',
|
||||
},
|
||||
],
|
||||
{ strict: true },
|
||||
);
|
||||
|
||||
|
|
@ -2084,6 +2095,38 @@ describe('SessionArtifactStore', () => {
|
|||
});
|
||||
expect(artifact).not.toHaveProperty('workspacePath');
|
||||
expect(artifact).not.toHaveProperty('sizeBytes');
|
||||
|
||||
const snapshot = (
|
||||
store as unknown as {
|
||||
buildSnapshotPayload(
|
||||
recordedAt: string,
|
||||
sequence: number,
|
||||
): SessionArtifactSnapshotRecordPayload;
|
||||
}
|
||||
).buildSnapshotPayload('2026-07-06T00:00:00.000Z', 1);
|
||||
expect(snapshot.artifacts[0]).toMatchObject({
|
||||
storage: 'workspace',
|
||||
status: 'missing',
|
||||
workspacePath: 'report.txt',
|
||||
});
|
||||
|
||||
await fs.rm(path.join(workspace, 'report.txt'));
|
||||
const restored = new SessionArtifactStore({
|
||||
sessionId: 's7-symlink-refresh',
|
||||
workspaceCwd: workspace,
|
||||
persistence,
|
||||
});
|
||||
await restored.restore({
|
||||
...snapshot,
|
||||
tombstonedIds: snapshot.tombstonedIds ?? [],
|
||||
stickyEphemeralIds: snapshot.stickyEphemeralIds ?? [],
|
||||
warnings: [],
|
||||
});
|
||||
expect((await restored.list()).artifacts[0]).toMatchObject({
|
||||
storage: 'workspace',
|
||||
status: 'missing',
|
||||
workspacePath: 'report.txt',
|
||||
});
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
await fs.rm(outside, { recursive: true, force: true });
|
||||
|
|
|
|||
|
|
@ -217,6 +217,7 @@ interface NormalizedArtifact extends DaemonSessionArtifact {
|
|||
interface StoredArtifact extends NormalizedArtifact {
|
||||
insertSeq: number;
|
||||
durableTombstoneRequired?: boolean;
|
||||
hideWorkspacePath?: boolean;
|
||||
}
|
||||
|
||||
interface WorkspaceStatusExpected {
|
||||
|
|
@ -867,9 +868,7 @@ export class SessionArtifactStore {
|
|||
const artifacts = Array.from(this.artifacts.values())
|
||||
.filter((artifact) => artifact.retention !== 'ephemeral')
|
||||
.sort((a, b) => a.insertSeq - b.insertSeq)
|
||||
.map((artifact) =>
|
||||
toPersistedArtifact(toPublicArtifact(artifact), recordedAt),
|
||||
);
|
||||
.map((artifact) => toPersistedArtifact(artifact, recordedAt));
|
||||
const stickyEphemeralIds = Array.from(this.stickyEphemeralIds);
|
||||
return {
|
||||
v: SESSION_ARTIFACT_PERSISTENCE_VERSION,
|
||||
|
|
@ -1241,7 +1240,7 @@ export class SessionArtifactStore {
|
|||
if (status.escaped) {
|
||||
artifact.status = 'missing';
|
||||
artifact.sizeBytes = undefined;
|
||||
delete artifact.workspacePath;
|
||||
artifact.hideWorkspacePath = true;
|
||||
}
|
||||
artifact.lastStatAt = options.now ?? Date.now();
|
||||
} catch (error) {
|
||||
|
|
@ -1457,6 +1456,12 @@ function mergeArtifact(
|
|||
publishedUpdate || existing.storage === 'published'
|
||||
? undefined
|
||||
: (existing.workspacePath ?? incoming.workspacePath),
|
||||
hideWorkspacePath:
|
||||
publishedUpdate ||
|
||||
existing.storage === 'published' ||
|
||||
incoming.workspacePath
|
||||
? undefined
|
||||
: existing.hideWorkspacePath,
|
||||
mimeType: publishedUpdate
|
||||
? (incoming.mimeType ?? existing.mimeType)
|
||||
: existing.mimeType,
|
||||
|
|
@ -1490,6 +1495,7 @@ function mergeArtifact(
|
|||
next.title = incoming.title;
|
||||
next.description = incoming.description;
|
||||
delete next.workspacePath;
|
||||
delete next.hideWorkspacePath;
|
||||
}
|
||||
|
||||
const changed = !publicArtifactsEqual(
|
||||
|
|
@ -1711,6 +1717,8 @@ function removePriorChange(
|
|||
function toPublicArtifact(
|
||||
artifact: StoredArtifact | DaemonSessionArtifact,
|
||||
): DaemonSessionArtifact {
|
||||
const hideWorkspacePath =
|
||||
'hideWorkspacePath' in artifact && artifact.hideWorkspacePath === true;
|
||||
const {
|
||||
id,
|
||||
kind,
|
||||
|
|
@ -1745,7 +1753,7 @@ function toPublicArtifact(
|
|||
status,
|
||||
title,
|
||||
...(description ? { description } : {}),
|
||||
...(workspacePath ? { workspacePath } : {}),
|
||||
...(workspacePath && !hideWorkspacePath ? { workspacePath } : {}),
|
||||
...(managedId ? { managedId } : {}),
|
||||
...(url ? { url } : {}),
|
||||
...(mimeType ? { mimeType } : {}),
|
||||
|
|
|
|||
|
|
@ -401,6 +401,9 @@ class FakeBridge {
|
|||
}
|
||||
| undefined;
|
||||
lastArtifactListSessionId: string | undefined;
|
||||
lastArtifactListContext:
|
||||
| Parameters<HttpAcpBridge['getSessionArtifacts']>[1]
|
||||
| undefined;
|
||||
lastRemovedArtifact:
|
||||
| {
|
||||
sessionId: string;
|
||||
|
|
@ -408,8 +411,12 @@ class FakeBridge {
|
|||
context: Parameters<HttpAcpBridge['removeSessionArtifact']>[2];
|
||||
}
|
||||
| undefined;
|
||||
async getSessionArtifacts(sessionId: string) {
|
||||
async getSessionArtifacts(
|
||||
sessionId: string,
|
||||
context: Parameters<HttpAcpBridge['getSessionArtifacts']>[1],
|
||||
) {
|
||||
this.lastArtifactListSessionId = sessionId;
|
||||
this.lastArtifactListContext = context;
|
||||
return {
|
||||
v: 1,
|
||||
sessionId,
|
||||
|
|
@ -6042,6 +6049,10 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
|
|||
},
|
||||
});
|
||||
expect(bridge.lastArtifactListSessionId).toBe('sess-1');
|
||||
expect(bridge.lastArtifactListContext).toEqual({
|
||||
clientId: 'client-1',
|
||||
fromLoopback: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('_qwen/session/artifacts/add forwards only public artifact fields', async () => {
|
||||
|
|
@ -6098,6 +6109,10 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
|
|||
expect(artifact).not.toHaveProperty('clientId');
|
||||
expect(artifact).not.toHaveProperty('toolName');
|
||||
expect(artifact).not.toHaveProperty('hookEventName');
|
||||
expect(bridge.lastAddedArtifact?.context).toEqual({
|
||||
clientId: 'client-1',
|
||||
fromLoopback: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('_qwen/session/artifacts/add maps artifact validation errors to invalid params', async () => {
|
||||
|
|
@ -6173,6 +6188,10 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
|
|||
expect(bridge.lastRemovedArtifact).toMatchObject({
|
||||
sessionId: 'sess-1',
|
||||
artifactId: 'artifact-1',
|
||||
context: {
|
||||
clientId: 'client-1',
|
||||
fromLoopback: true,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -2721,6 +2721,71 @@ describe('SessionService', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('does not treat trailing artifact side records as the fork leaf', async () => {
|
||||
const oldId = '73737373-7373-7373-7373-737373737373';
|
||||
const newId = '83838383-8383-8383-8383-838383838383';
|
||||
const { file, lines } = seedSession(oldId);
|
||||
const url = 'https://example.com/trailing-forked';
|
||||
const oldArtifactId = stableSessionArtifactId(oldId, `url:${url}`);
|
||||
const artifactRecord = {
|
||||
uuid: 'artifact-tail',
|
||||
parentUuid: 'u2',
|
||||
sessionId: oldId,
|
||||
type: 'system',
|
||||
subtype: 'session_artifact_event',
|
||||
timestamp: '2026-04-22T00:00:01.500Z',
|
||||
cwd,
|
||||
version: 'test',
|
||||
systemPayload: {
|
||||
v: SESSION_ARTIFACT_PERSISTENCE_VERSION,
|
||||
sessionId: oldId,
|
||||
sequence: 1,
|
||||
recordedAt: '2026-04-22T00:00:01.500Z',
|
||||
changes: [
|
||||
{
|
||||
action: 'created',
|
||||
artifactId: oldArtifactId,
|
||||
artifact: {
|
||||
id: oldArtifactId,
|
||||
kind: 'link',
|
||||
storage: 'external_url',
|
||||
source: 'client',
|
||||
status: 'available',
|
||||
title: 'Trailing forked artifact',
|
||||
url,
|
||||
retention: 'restorable',
|
||||
clientRetained: true,
|
||||
createdAt: '2026-04-22T00:00:01.500Z',
|
||||
updatedAt: '2026-04-22T00:00:01.500Z',
|
||||
persistedAt: '2026-04-22T00:00:01.500Z',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
fs.writeFileSync(
|
||||
file,
|
||||
[...lines, artifactRecord]
|
||||
.map((line) => JSON.stringify(line))
|
||||
.join('\n') + '\n',
|
||||
);
|
||||
|
||||
const result = await service.forkSession(oldId, newId);
|
||||
const loaded = await service.loadSession(newId);
|
||||
|
||||
expect(result.copiedCount).toBe(3);
|
||||
expect(
|
||||
loaded?.conversation.messages.map((record) => record.uuid),
|
||||
).toEqual(['u1', 'u2']);
|
||||
expect(loaded?.lastCompletedUuid).toBe('u2');
|
||||
expect(loaded?.artifactSnapshot?.artifacts).toEqual([
|
||||
expect.objectContaining({
|
||||
id: stableSessionArtifactId(newId, `url:${url}`),
|
||||
title: 'Trailing forked artifact',
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it('does not resurrect artifacts removed by later side records when forking', async () => {
|
||||
const oldId = '72727272-7272-7272-7272-727272727272';
|
||||
const newId = '82828282-8282-8282-8282-828282828282';
|
||||
|
|
|
|||
|
|
@ -1466,7 +1466,13 @@ export class SessionService {
|
|||
|
||||
// Copy only the active branch. Rewind leaves old records in the JSONL as
|
||||
// abandoned parentUuid branches; copying raw records would resurrect them.
|
||||
const { messages: activeMessages } = this.reconstructHistory(records);
|
||||
const leafUuid = this.lastConversationRecordUuid(records);
|
||||
if (!leafUuid) {
|
||||
throw new Error(`Source session not found or empty: ${sourceSessionId}`);
|
||||
}
|
||||
const { messages: activeMessages } = this.reconstructHistory(records, {
|
||||
leafUuid,
|
||||
});
|
||||
const sourceRecords = includeActiveSideArtifactRecords(
|
||||
records,
|
||||
activeMessages,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue