Преглед на файлове

fix: enforce interview document ownership

Alvin Unreal преди 1 месец
родител
ревизия
c6da3ae980
променени са 3 файла, в които са добавени 201 реда и са изтрити 76 реда
  1. 77 15
      src/interview/document.ts
  2. 103 59
      src/interview/finalization.test.ts
  3. 21 2
      src/interview/service.ts

+ 77 - 15
src/interview/document.ts

@@ -26,6 +26,16 @@ type DocumentLock = {
   token: string;
 };
 
+export class InterviewDocumentOwnershipError extends Error {
+  constructor(
+    readonly markdownPath: string,
+    readonly ownerSessionID: string,
+  ) {
+    super(`Interview document is owned by another session: ${ownerSessionID}`);
+    this.name = 'InterviewDocumentOwnershipError';
+  }
+}
+
 function isNoSuchFileError(error: unknown): boolean {
   return (error as NodeJS.ErrnoException).code === 'ENOENT';
 }
@@ -138,6 +148,67 @@ export async function withInterviewDocumentLock<T>(
   }
 }
 
+function buildInterviewFrontmatter(
+  sessionID: string,
+  baseMessageCount: number,
+  owner = 'agent',
+  tags = ['spec', 'diagnostic'],
+): string {
+  const now = new Date();
+  const dateStr = now.toISOString().split('T')[0];
+  return [
+    '---',
+    `sessionID: ${sessionID}`,
+    `baseMessageCount: ${baseMessageCount}`,
+    `updatedAt: ${now.toISOString()}`,
+    'version: 1.0',
+    `date_created: ${dateStr}`,
+    `owner: ${owner}`,
+    `tags: [${tags.join(', ')}]`,
+    '---',
+    '',
+  ].join('\n');
+}
+
+export async function claimInterviewDocument(
+  markdownPath: string,
+  sessionID: string,
+  baseMessageCount: number,
+): Promise<string> {
+  return withInterviewDocumentLock(markdownPath, async () => {
+    const document = await fs.readFile(markdownPath, 'utf8');
+    const frontmatter = sharedParseFrontmatter(document);
+    const owner = frontmatter?.sessionID;
+    if (owner && owner !== sessionID) {
+      throw new InterviewDocumentOwnershipError(markdownPath, owner);
+    }
+    if (owner) {
+      return document;
+    }
+
+    const frontmatterMatch = document.match(
+      /^---\r?\n([\s\S]*?)\r?\n---\r?\n?/,
+    );
+    const existingFrontmatter = frontmatterMatch?.[1]
+      .split(/\r?\n/)
+      .filter((line) => !/^sessionID\s*:/i.test(line))
+      .join('\n');
+    const next = frontmatterMatch
+      ? [
+          '---',
+          `sessionID: ${sessionID}`,
+          `baseMessageCount: ${baseMessageCount}`,
+          existingFrontmatter,
+          '---',
+          '',
+          document.slice(frontmatterMatch[0].length),
+        ].join('\n')
+      : `${buildInterviewFrontmatter(sessionID, baseMessageCount)}${document}`;
+    await fs.writeFile(markdownPath, next, 'utf8');
+    return next;
+  });
+}
+
 export function normalizeOutputFolder(outputFolder: string): string {
   const normalized = outputFolder.trim().replace(/^\/+|\/+$/g, '');
   return normalized || DEFAULT_OUTPUT_FOLDER;
@@ -276,25 +347,16 @@ export function buildInterviewDocument(
   const normalizedSummary = summary.trim() || 'Waiting for interview answers.';
   const normalizedHistory = history.trim() || 'No answers yet.';
 
-  const now = new Date();
-  const dateStr = now.toISOString().split('T')[0];
-
   const owner = meta?.owner ?? 'agent';
   const tags = meta?.tags ?? ['spec', 'diagnostic'];
 
   const frontmatter = meta?.sessionID
-    ? [
-        '---',
-        `sessionID: ${meta.sessionID}`,
-        `baseMessageCount: ${meta.baseMessageCount ?? 0}`,
-        `updatedAt: ${now.toISOString()}`,
-        `version: 1.0`,
-        `date_created: ${dateStr}`,
-        `owner: ${owner}`,
-        `tags: [${tags.join(', ')}]`,
-        '---',
-        '',
-      ].join('\n')
+    ? buildInterviewFrontmatter(
+        meta.sessionID,
+        meta.baseMessageCount ?? 0,
+        owner,
+        tags,
+      )
     : '';
 
   return [

+ 103 - 59
src/interview/finalization.test.ts

@@ -205,7 +205,7 @@ describe('interview finalization', () => {
     await fs.rm(directory, { recursive: true, force: true });
   });
 
-  test('serializes overlapping writes from service instances sharing one document', async () => {
+  test('prevents a second session from resuming the owned document', async () => {
     const directory = await fs.mkdtemp('/tmp/interview-resume-lock-');
     const documentPath = path.join(directory, 'interview', 'shared.md');
     await fs.mkdir(path.dirname(documentPath), { recursive: true });
@@ -215,75 +215,119 @@ describe('interview finalization', () => {
       'utf8',
     );
 
-    const scenarios = await Promise.all(
-      [
-        {
-          sessionID: 'ses_one',
-          summary: 'One draft',
-          questionId: 'q-one',
-          question: 'One?',
-          answer: 'One answer',
+    const firstMessages: InterviewMessage[] = [];
+    const firstService = createInterviewService(
+      { directory } as never,
+      undefined,
+      {
+        runtime: {
+          messages: async () => firstMessages,
+          notify: async () => {},
+          continue: async () => {},
+          rename: async () => {},
         },
+        openBrowser: () => {},
+      },
+    );
+    firstService.setBaseUrlResolver(async () => 'http://127.0.0.1:43211');
+    await firstService.handleCommandExecuteBefore(
+      {
+        command: 'interview',
+        sessionID: 'ses_one',
+        arguments: documentPath,
+      },
+      { parts: [] },
+    );
+    firstMessages.push({
+      info: { role: 'assistant' },
+      parts: [
         {
-          sessionID: 'ses_two',
-          summary: 'Two draft',
-          questionId: 'q-two',
-          question: 'Two?',
-          answer: 'Two answer',
+          type: 'text',
+          text: '<interview_state>{"summary":"One draft","title":"Shared Title","questions":[{"id":"q-one","question":"One?","options":["Yes"]}]}</interview_state>',
         },
-      ].map(async (scenario) => {
-        const messages: InterviewMessage[] = [];
-        const runtime: InterviewSessionRuntime = {
-          messages: async () => messages,
+      ],
+    });
+    const firstInterviewID = firstService.getActiveInterviewId('ses_one');
+    expect(firstInterviewID).not.toBeNull();
+    await firstService.getInterviewState(firstInterviewID as string);
+    const ownedDocument = await fs.readFile(documentPath, 'utf8');
+
+    const secondService = createInterviewService(
+      { directory } as never,
+      undefined,
+      {
+        runtime: {
+          messages: async () => [],
           notify: async () => {},
           continue: async () => {},
           rename: async () => {},
-        };
-        const service = createInterviewService(
-          { directory } as never,
-          undefined,
-          { runtime, openBrowser: () => {} },
-        );
-        service.setBaseUrlResolver(async () => 'http://127.0.0.1:43211');
-        await service.handleCommandExecuteBefore(
-          {
-            command: 'interview',
-            sessionID: scenario.sessionID,
-            arguments: documentPath,
-          },
-          { parts: [] },
-        );
-        messages.push({
-          info: { role: 'assistant' },
-          parts: [
-            {
-              type: 'text',
-              text: `<interview_state>{"summary":"${scenario.summary}","title":"Shared Title","questions":[{"id":"${scenario.questionId}","question":"${scenario.question}","options":["Yes"]}]}</interview_state>`,
-            },
-          ],
-        });
-        const interviewID = service.getActiveInterviewId(scenario.sessionID);
-        expect(interviewID).not.toBeNull();
-        return { ...scenario, service, interviewID: interviewID as string };
-      }),
+        },
+        openBrowser: () => {},
+      },
+    );
+    secondService.setBaseUrlResolver(async () => 'http://127.0.0.1:43211');
+    const secondOutput = {
+      parts: [] as Array<{ type: string; text?: string }>,
+    };
+    await secondService.handleCommandExecuteBefore(
+      {
+        command: 'interview',
+        sessionID: 'ses_two',
+        arguments: documentPath,
+      },
+      secondOutput,
     );
 
-    await Promise.all(
-      scenarios.map(({ service, interviewID }) =>
-        service.getInterviewState(interviewID),
-      ),
+    expect(secondService.getActiveInterviewId('ses_two')).toBeNull();
+    expect(secondOutput.parts[0]?.text).toContain('already owned');
+    expect(await fs.readFile(documentPath, 'utf8')).toBe(ownedDocument);
+    expect(await fs.readdir(path.dirname(documentPath))).toEqual(['shared.md']);
+
+    await fs.rm(directory, { recursive: true, force: true });
+  });
+
+  test('allows the owning session to resume through another service instance', async () => {
+    const directory = await fs.mkdtemp('/tmp/interview-owner-resume-');
+    const documentPath = path.join(directory, 'interview', 'owned.md');
+    await fs.mkdir(path.dirname(documentPath), { recursive: true });
+    await fs.writeFile(documentPath, '# Owned\n\nDraft.', 'utf8');
+
+    const createService = () =>
+      createInterviewService({ directory } as never, undefined, {
+        runtime: {
+          messages: async () => [],
+          notify: async () => {},
+          continue: async () => {},
+          rename: async () => {},
+        },
+        openBrowser: () => {},
+      });
+    const firstService = createService();
+    const secondService = createService();
+    firstService.setBaseUrlResolver(async () => 'http://127.0.0.1:43211');
+    secondService.setBaseUrlResolver(async () => 'http://127.0.0.1:43211');
+
+    await firstService.handleCommandExecuteBefore(
+      {
+        command: 'interview',
+        sessionID: 'same-session',
+        arguments: documentPath,
+      },
+      { parts: [] },
     );
-    await Promise.all(
-      scenarios.map(({ service, interviewID, questionId, answer }) =>
-        service.submitAnswers(interviewID, [{ questionId, answer }]),
-      ),
+    await secondService.handleCommandExecuteBefore(
+      {
+        command: 'interview',
+        sessionID: 'same-session',
+        arguments: documentPath,
+      },
+      { parts: [] },
     );
 
-    const document = await fs.readFile(documentPath, 'utf8');
-    expect(document).toContain('# Shared Title');
-    expect(document).toContain('A: One answer');
-    expect(document).toContain('A: Two answer');
-    expect(await fs.readdir(path.dirname(documentPath))).toEqual(['shared.md']);
+    expect(secondService.getActiveInterviewId('same-session')).not.toBeNull();
+    expect(await fs.readFile(documentPath, 'utf8')).toContain(
+      'sessionID: same-session',
+    );
 
     await fs.rm(directory, { recursive: true, force: true });
   });

+ 21 - 2
src/interview/service.ts

@@ -12,12 +12,14 @@ import {
 import { parseModelReference } from '../utils/session';
 import {
   appendInterviewAnswers,
+  claimInterviewDocument,
   createInterviewDirectoryPath,
   createInterviewFilePath,
   DEFAULT_OUTPUT_FOLDER,
   ensureInterviewFile,
   extractSummarySection,
   extractTitle,
+  InterviewDocumentOwnershipError,
   normalizeOutputFolder,
   parseSpecBlocks,
   readInterviewDocument,
@@ -364,8 +366,12 @@ export function createInterviewService(
       }
     }
 
-    const document = await fs.readFile(markdownPath, 'utf8');
     const messages = await loadMessages(sessionID);
+    const document = await claimInterviewDocument(
+      markdownPath,
+      sessionID,
+      messages.length,
+    );
     const title = extractTitle(document);
     const record: InterviewRecord = {
       id: randomUUID(),
@@ -664,7 +670,20 @@ export function createInterviewService(
       idea,
     );
     if (resumePath) {
-      const interview = await resumeInterview(input.sessionID, resumePath);
+      let interview: InterviewRecord;
+      try {
+        interview = await resumeInterview(input.sessionID, resumePath);
+      } catch (error) {
+        if (error instanceof InterviewDocumentOwnershipError) {
+          output.parts.push(
+            createInternalAgentTextPart(
+              'This interview document is already owned by another OpenCode session and cannot be resumed here.',
+            ),
+          );
+          return;
+        }
+        throw error;
+      }
       const document = await fs.readFile(interview.markdownPath, 'utf8');
       await notifyInterviewUrl(input.sessionID, interview);
       output.parts.push(