fix(gh-aw): bind implementation acceptance to selected plan revision (#2206)

test/gh-aw-activation-finalization.test.ts

@@ -118,7 +118,11 @@ function fixture({ count = 2, epic = false } = {}) {
   const comments = new Map<number, ReturnType<typeof comment>>([[10, comment(10, planBody)]]);
   if (epic) {
     comments.set(7, comment(7, block('Structured data', envelope('scope-accepted'))));
-    comments.set(8, comment(8, block('Structured data', envelope('impl-accepted'))));
+    comments.set(8, comment(8, [
+      '## Implementation Accepted',
+      `- **Implementation plan version:** ${comments.get(10)!.html_url}`,
+      block('Structured data', envelope('impl-accepted')),
+    ].join('\n')));
   }
   function issue(number: number, values: Record<string, any> = {}) {
     return {
@@ -1603,6 +1607,24 @@ describe('combined implementation-phase acceptance and activation', () => {
     await expect(f.create(capture)).rejects.toThrow();
     expect(f.writes).toEqual([]);
   });
+
+  it('rejects activation when implementation acceptance references an older plan revision (#2206)', async () => {
+    const f = fixture({ epic: true });
+    f.comments.set(20, f.comment(20, f.comments.get(10)!.body));
+    writeFileSync(f.outputPath, JSON.stringify({ items: [{ type: 'activate_plan', plan_comment_id: '20' }] }));
+    await expect(f.prepare()).rejects.toThrow(/implementation acceptance must uniquely reference the selected implementation plan revision/);
+    expect(f.writes).toEqual([]);
+    expect(f.createIssue).not.toHaveBeenCalled();
+
+    f.comments.set(21, f.comment(21, [
+      '## Implementation Accepted',
+      `- **Implementation plan version:** ${f.comments.get(20)!.html_url}`,
+      block('Structured data', envelope('impl-accepted')),
+    ].join('\n')));
+    const capture = await f.prepare();
+    expect(capture.plan_comment.id).toBe(20);
+    expect(capture.approvals.implementation.id).toBe(21);
+  });
 });
 
 describe('explicit pending-source current-state reconciliation', () => {

test/gh-aw-activation-label-provisioning.test.ts

@@ -84,7 +84,11 @@ function fixture({ owners = ['Builder', 'Reviewer', '@copilot'], implementation
   const comments = new Map([[10, comment(10, planBody)]]);
   if (implementation) {
     comments.set(7, comment(7, block('scope-accepted')));
-    comments.set(8, comment(8, block('impl-accepted')));
+    comments.set(8, comment(8, [
+      '## Implementation Accepted',
+      `- **Implementation plan version:** ${comments.get(10)!.html_url}`,
+      block('impl-accepted'),
+    ].join('\n')));
   }
   const issue = (number: number, params: any = {}) => ({
     number, id: number + 1000, node_id: `I_labels_${number}`,

test/gh-aw-activation-summary-outcomes.test.ts

@@ -83,7 +83,11 @@ function fixture(command: string = 'activate', secondOwner = 'Kint') {
   const comments = new Map([[10, comment(10, planBody)]]);
   if (granular) {
     comments.set(7, comment(7, block('Structured data', envelope('scope-accepted'))));
-    comments.set(8, comment(8, block('Structured data', envelope('impl-accepted'))));
+    comments.set(8, comment(8, [
+      '## Implementation Accepted',
+      `- **Implementation plan version:** ${comments.get(10)!.html_url}`,
+      block('Structured data', envelope('impl-accepted')),
+    ].join('\n')));
   }
   const issue = (number: number, values: Record<string, any> = {}) => ({
     number, id: number + 1000, node_id: `I_${number}`, body: '', labels: [],

test/gh-aw-plan-lifecycle.test.ts

@@ -232,7 +232,7 @@ function activationFixture(body = taskTable(), command = 'activate', kind = 'pla
   });
   const artifacts = (kind: string) => [...comments.values()].filter(item =>
     parseStructuredData(item.body)?.squad_artifact === kind);
-  return { prepare, create, finalize, artifacts, comments, comment, issues, createIssue, github, context };
+  return { prepare, create, finalize, artifacts, comments, comment, issues, createIssue, github, context, root, outputPath };
 }
 
 // ---------------------------------------------------------------------------
@@ -579,7 +579,11 @@ describe('#1758.1: squad-plan-accept routes granular plans before failing', () =
     f.comments.set(7, f.comment(7, structured('scope-accepted')));
     await expect(f.prepare()).rejects.toThrow(/granular activation requires existing impl-accepted/);
     expect(f.createIssue).not.toHaveBeenCalled();
-    f.comments.set(8, f.comment(8, structured('impl-accepted')));
+    f.comments.set(8, f.comment(8, [
+      '## Implementation Accepted',
+      `- **Implementation plan version:** ${f.comments.get(10)!.html_url}`,
+      structured('impl-accepted'),
+    ].join('\n')));
     const capture = await f.prepare();
     expect(capture.plan.kind).toBe('implementation');
     expect(capture.operations.map((operation: any) => operation.id)).toEqual(['epic:epic1', 'task:1']);
@@ -588,6 +592,85 @@ describe('#1758.1: squad-plan-accept routes granular plans before failing', () =
     expect(f.artifacts('activated')).toHaveLength(1);
   });
 
+  it('binds implementation acceptance to the selected plan revision and rejects stale, missing, or ambiguous links (#2206)', async () => {
+    // 1. Reproduce exact #2206 scenario: plan 10 accepted by comment 13, then newer unaccepted plan 20 is added
+    const f = activationFixture(implementation, 'plan activate', 'implementation');
+    f.comments.set(11, f.comment(11, structured('scope-accepted')));
+    f.comments.set(13, f.comment(13, [
+      '## Implementation Accepted',
+      `- **Implementation plan version:** ${f.comments.get(10)!.html_url}`,
+      structured('impl-accepted'),
+    ].join('\n')));
+    f.comments.set(20, f.comment(20, `${implementation}\n${structured('implementation')}`));
+    writeFileSync(f.outputPath, JSON.stringify({ items: [{ type: 'activate_plan', plan_comment_id: '20' }] }));
+    await expect(f.prepare()).rejects.toThrow(/implementation acceptance must uniquely reference the selected implementation plan revision/);
+    expect(f.createIssue).not.toHaveBeenCalled();
+    expect(f.github.rest.issues.createComment).not.toHaveBeenCalled();
+
+    // 2. Missing link in impl-accepted rejects
+    f.comments.set(13, f.comment(13, structured('impl-accepted')));
+    await expect(f.prepare()).rejects.toThrow(/implementation acceptance must uniquely reference the selected implementation plan revision/);
+
+    // 3. Ambiguous links (both 10 and 20 referenced) rejects
+    f.comments.set(13, f.comment(13, [
+      '## Implementation Accepted',
+      `- **Implementation plan version:** ${f.comments.get(10)!.html_url} and ${f.comments.get(20)!.html_url}`,
+      structured('impl-accepted'),
+    ].join('\n')));
+    await expect(f.prepare()).rejects.toThrow(/implementation acceptance must uniquely reference the selected implementation plan revision/);
+
+    // 4. Matching acceptance for plan 20 (via standard comment-id or URL format) succeeds
+    f.comments.set(21, f.comment(21, [
+      '## Implementation Accepted',
+      '- **Implementation plan version:** #42 (comment-id: 20)',
+      structured('impl-accepted'),
+    ].join('\n')));
+    const capture = await f.prepare();
+    expect(capture.plan_comment.id).toBe(20);
+    expect(capture.approvals.implementation.id).toBe(21);
+  });
+
+  it('binds phased implementation acceptance to the selected plan revision and rejects stale phase acceptance (#2206)', async () => {
+    const phasedImpl = [
+      '### Phase 1',
+      taskTable(['| 1 | Phase 1 task | Builder | builder | None | epic1 |'], true),
+    ].join('\n');
+    const f = activationFixture(phasedImpl, 'plan activate phase 1', 'implementation');
+    f.comments.set(11, f.comment(11, structured('scope-accepted')));
+    f.comments.set(13, f.comment(13, [
+      '## Implementation Phase 1 Accepted',
+      `**Implementation plan:** ${f.comments.get(10)!.html_url}`,
+      `**Implementation SHA-256:** ${digest(f.comments.get(10)!.body)}`,
+      structured('impl-phases-accepted', [1]),
+    ].join('\n')));
+
+    // Add newer implementation revision 20 without accepting phase 1 for revision 20
+    f.comments.set(20, f.comment(20, `${phasedImpl}\n${structured('implementation')}`));
+    writeFileSync(f.outputPath, JSON.stringify({ items: [{ type: 'activate_plan', plan_comment_id: '20' }] }));
+    await expect(f.prepare()).rejects.toThrow(/implementation acceptance must uniquely reference the selected implementation plan revision/);
+    expect(f.createIssue).not.toHaveBeenCalled();
+
+    // Even if URL points to 20, a stale SHA-256 on impl-phases-accepted rejects
+    f.comments.set(14, f.comment(14, [
+      '## Implementation Phase 1 Accepted',
+      `**Implementation plan:** ${f.comments.get(20)!.html_url}`,
+      `**Implementation SHA-256:** ${'0'.repeat(64)}`,
+      structured('impl-phases-accepted', [1]),
+    ].join('\n')));
+    await expect(f.prepare()).rejects.toThrow(/implementation acceptance SHA-256 does not match/);
+
+    // Matching URL and SHA-256 for revision 20 succeeds
+    f.comments.set(15, f.comment(15, [
+      '## Implementation Phase 1 Accepted',
+      `**Implementation plan:** ${f.comments.get(20)!.html_url}`,
+      `**Implementation SHA-256:** ${digest(f.comments.get(20)!.body)}`,
+      structured('impl-phases-accepted', [1]),
+    ].join('\n')));
+    const capture = await f.prepare();
+    expect(capture.plan_comment.id).toBe(20);
+    expect(capture.approvals.implementation.id).toBe(15);
+  });
+
   it('fails closed without a supported task plan and never turns a program into invented tasks', async () => {
     for (const kind of ['program', 'research']) {
       const f = activationFixture('No implementation task table exists.', 'activate', kind);

workflows/shared/squad-activation.mjs

@@ -553,12 +553,13 @@ export async function prepareActivation({ github, context, env, root, outputPath
       validateImplementationVerdict(validation.body, planComment, fullPlan);
       approvals = { scope, validation };
     } else {
-      const implementation = acceptedComments.find(comment => {
+      const implementation = acceptedComments.filter(comment => {
         const value = envelope(comment.body);
         return value.squad_artifact === 'impl-accepted' ||
           (command.phase !== null && value.squad_artifact === 'impl-phases-accepted' && value.phases?.includes(command.phase));
-      });
+      }).at(-1);
       requireThat(implementation, 'granular activation requires existing impl-accepted or selected implementation-phase acceptance');
+      validateImplementationAcceptance(implementation, planComment, `${context.repo.owner}/${context.repo.repo}`, origin);
       approvals = { scope, implementation };
     }
   }
@@ -683,6 +684,47 @@ function validationTable(body, columns) {
   return rows;
 }
 
+function extractImplementationPlanCommentIds(body, repository, origin) {
+  const text = String(body ?? '')
+    .replace(/<!--[\s\S]*?-->/g, '')
+    .replace(/```[\s\S]*?```/g, '');
+  const escapedRepo = repository.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+  const ids = [];
+  const collect = source => {
+    for (const match of source.matchAll(
+      new RegExp(`https://github\\.com/${escapedRepo}/issues/${origin}#issuecomment-([1-9]\\d*)\\b`, 'g'),
+    )) {
+      ids.push(Number(match[1]));
+    }
+    for (const match of source.matchAll(/(?:^|[^A-Za-z0-9_/])#issuecomment-([1-9]\d*)\b/g)) {
+      ids.push(Number(match[1]));
+    }
+    for (const match of source.matchAll(/\bcomment(?:[-_\s]+id)?\s*[:=#]?\s*([1-9]\d*)\b/gi)) {
+      ids.push(Number(match[1]));
+    }
+  };
+  const labeledLines = text.split(/\r?\n/).filter(line =>
+    /^(?:[-*]\s+)?(?:\*\*)?(?:implementation(?:\s+plan)?(?:\s+version)?|impl\s+plan(?:\s+version)?|plan\s+version)(?:\*\*)?\s*:/i
+      .test(line.trim()),
+  );
+  if (labeledLines.length) {
+    for (const line of labeledLines) collect(line);
+  } else {
+    collect(text);
+  }
+  return [...new Set(ids.filter(positive))];
+}
+
+function validateImplementationAcceptance(comment, planComment, repository, origin) {
+  const body = String(comment.body ?? '');
+  const ids = extractImplementationPlanCommentIds(body, repository, origin);
+  requireThat(ids.length === 1 && ids[0] === planComment.id,
+    'implementation acceptance must uniquely reference the selected implementation plan revision');
+  const shas = [...body.matchAll(/^\*\*Implementation SHA-256:\*\*\s*(\S+)\s*$/gm)].map(match => match[1]);
+  requireThat(shas.length <= 1 && (shas.length === 0 || shas[0] === digest(planComment.body)),
+    'implementation acceptance SHA-256 does not match the selected implementation plan revision');
+}
+
 function validateImplementationVerdict(body, planComment, plan) {
   const verdicts = [...body.matchAll(/^RESULT:\s*(PASS|FAIL)\s*$/gm)];
   requireThat(verdicts.length === 1 && verdicts[0][1] === 'PASS' &&

workflows/squad-workflows.manifest.json

@@ -144,7 +144,7 @@
       "package_destination": ".github/workflows/shared/squad-activation.mjs",
       "destination": ".github/workflows/shared/squad-activation.mjs",
       "ownership": "manifest",
-      "sha256": "16c579cc85ff191614a9635757ee5281f4f9f3f1f0f8ead44fe33c44b90a0980"
+      "sha256": "1d6cc4f5a924d471e83b4bd0cfb754fe8e5376efea339d2a303138350c3bc4f8"
     },
     {
       "path": "shared/check-agent-binding.mjs",