fix(#3096): enforce sequential Steps 7+8 + Edit-only tool discipline in ai-integration-phase (#3143)
* fix(#3096): enforce sequential Steps 7+8 + Edit-only discipline in ai-integration-phase Root cause: Steps 7 (gsd-ai-researcher) and 8 (gsd-domain-researcher) were listed without an explicit sequential constraint. An orchestrator optimizing for speed could parallelize them since sections appeared disjoint. gsd-domain-researcher's Write at finalization replaced the full AI-SPEC.md with its in-memory copy (pre-researcher state), losing Sections 3/4. Confirmed at 40% incidence (2/5 agents on a real run). Recovery cost: one extra ai-researcher dispatch, ~18 min wall. Fix: - Explicit 'MUST run sequentially' note on Step 7 (ordering note) - 'Wait for Step 7 to complete before spawning Step 8' on Step 8 - Edit-only tool discipline injected into both agent prompts: 'Use Edit exclusively - NEVER use Write on this file' prevents the last-writer-wins overwrite regardless of dispatch order Suite: 7043/7043. Closes #3096. * fix(lint): allow-test-rule for ai-integration-phase structural contract test
This commit is contained in:
5
.changeset/fix-3096-ai-integration-parallel-race.md
Normal file
5
.changeset/fix-3096-ai-integration-parallel-race.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3096
|
||||
---
|
||||
**`ai-integration-phase` Steps 7+8 now enforce sequential execution and Edit-only tool discipline** — when `gsd-ai-researcher` and `gsd-domain-researcher` were dispatched in parallel (an optimization an orchestrator could reasonably make since the sections appeared disjoint), `gsd-domain-researcher`'s `Write` call at finalization silently replaced the entire AI-SPEC.md with its pre-researcher copy, losing Sections 3/4. Confirmed at 40% incidence rate (2 of 5 agents on a real run). Fix adds an explicit sequential ordering note to Steps 7+8 ("MUST run sequentially — wait for Step 7 to complete before spawning Step 8") and injects Edit-only tool discipline into both agent prompts ("Use the Edit tool exclusively — NEVER use Write on this file"). Closes #3096.
|
||||
@@ -139,6 +139,8 @@ Fill in header fields:
|
||||
|
||||
## 7. Spawn gsd-ai-researcher
|
||||
|
||||
> **Ordering note (prevents tool-level last-writer-wins race):** Steps 7 and 8 write disjoint sections of AI-SPEC.md but MUST run sequentially — wait for Step 7 to complete before spawning Step 8. Both agents use the `Edit` tool exclusively (never `Write`) when modifying AI-SPEC.md. A `Write` on a shared file replaces the entire file, silently overwriting the other agent's work; `Edit` targets only the relevant lines. See #3096 for a confirmed 40%-incidence race on parallel dispatch.
|
||||
|
||||
Display:
|
||||
```
|
||||
◆ Step 2/4 — Researching {primary_framework} docs + AI systems best practices...
|
||||
@@ -148,9 +150,12 @@ Spawn `gsd-ai-researcher` with:
|
||||
```markdown
|
||||
Read ~/.claude/agents/gsd-ai-researcher.md for instructions.
|
||||
|
||||
**Tool discipline (mandatory):**
|
||||
Use the Edit tool exclusively when modifying AI-SPEC.md — NEVER use Write on this file.
|
||||
Write replaces the entire file and will overwrite work from parallel or sequential sibling agents.
|
||||
Before editing, verify the section you are about to write is still a template placeholder.
|
||||
|
||||
<objective>
|
||||
Research {primary_framework} for Phase {phase_number}: {phase_name}
|
||||
Write Sections 3 and 4 of AI-SPEC.md
|
||||
</objective>
|
||||
|
||||
<files_to_read>
|
||||
@@ -169,6 +174,8 @@ phase_context: Phase {phase_number}: {phase_name} — {phase_goal}
|
||||
|
||||
## 8. Spawn gsd-domain-researcher
|
||||
|
||||
> **Wait for Step 7 to complete before spawning this step** (see ordering note in Step 7).
|
||||
|
||||
Display:
|
||||
```
|
||||
◆ Step 3/4 — Researching domain context and expert evaluation criteria...
|
||||
@@ -178,9 +185,12 @@ Spawn `gsd-domain-researcher` with:
|
||||
```markdown
|
||||
Read ~/.claude/agents/gsd-domain-researcher.md for instructions.
|
||||
|
||||
**Tool discipline (mandatory):**
|
||||
Use the Edit tool exclusively when modifying AI-SPEC.md — NEVER use Write on this file.
|
||||
Write replaces the entire file and will overwrite work from parallel or sequential sibling agents.
|
||||
Before editing, verify the section you are about to write is still a template placeholder.
|
||||
|
||||
<objective>
|
||||
Research the business domain and expert evaluation criteria for Phase {phase_number}: {phase_name}
|
||||
Write Section 1b (Domain Context) of AI-SPEC.md
|
||||
</objective>
|
||||
|
||||
<files_to_read>
|
||||
|
||||
73
tests/bug-3096-ai-integration-phase-parallel-race.test.cjs
Normal file
73
tests/bug-3096-ai-integration-phase-parallel-race.test.cjs
Normal file
@@ -0,0 +1,73 @@
|
||||
'use strict';
|
||||
// allow-test-rule: reads product workflow markdown (ai-integration-phase.md) to verify structural ordering contract — not a source-grep test
|
||||
|
||||
// Regression guard for bug #3096.
|
||||
//
|
||||
// ai-integration-phase.md listed Steps 7+8 (gsd-ai-researcher +
|
||||
// gsd-domain-researcher) without an explicit sequential ordering constraint.
|
||||
// An orchestrator optimizing for speed could reasonably parallelize them
|
||||
// since the sections appeared disjoint. When parallelized, gsd-domain-researcher's
|
||||
// Write call at finalization replaced the whole AI-SPEC.md file with its
|
||||
// in-memory copy (pre-researcher state), silently overwriting Sections 3/4.
|
||||
//
|
||||
// Confirmed at 40% incidence rate on a real run (2 of 5 worktree agents hit it).
|
||||
// Recovery cost: one extra ai-researcher dispatch (~18 min wall).
|
||||
//
|
||||
// Fix:
|
||||
// 1. Explicit "MUST run sequentially" note on Steps 7 and 8
|
||||
// 2. Edit-only tool discipline injected into both agent prompts
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const src = fs.readFileSync(
|
||||
path.join(ROOT, 'get-shit-done', 'workflows', 'ai-integration-phase.md'),
|
||||
'utf8',
|
||||
);
|
||||
|
||||
describe('bug #3096: ai-integration-phase sequential ordering and Edit-only discipline', () => {
|
||||
test('Step 7 documents sequential ordering requirement', () => {
|
||||
assert.ok(
|
||||
src.includes('sequentially') || src.includes('sequential'),
|
||||
'Steps 7+8 ordering note is missing — parallel dispatch race can recur',
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 7 gsd-ai-researcher prompt includes Edit-only tool discipline', () => {
|
||||
// The discipline block must appear before </objective> for gsd-ai-researcher
|
||||
const step7Idx = src.indexOf('## 7. Spawn gsd-ai-researcher');
|
||||
const step8Idx = src.indexOf('## 8. Spawn gsd-domain-researcher');
|
||||
assert.ok(step7Idx !== -1, 'Step 7 not found');
|
||||
assert.ok(step8Idx !== -1, 'Step 8 not found');
|
||||
const step7Block = src.slice(step7Idx, step8Idx);
|
||||
assert.ok(
|
||||
step7Block.includes('Edit tool') && step7Block.includes('NEVER use Write'),
|
||||
'Step 7 agent prompt missing Edit-only tool discipline',
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 8 gsd-domain-researcher prompt includes Edit-only tool discipline', () => {
|
||||
const step8Idx = src.indexOf('## 8. Spawn gsd-domain-researcher');
|
||||
const step9Idx = src.indexOf('## 9. Spawn gsd-eval-planner');
|
||||
assert.ok(step8Idx !== -1, 'Step 8 not found');
|
||||
assert.ok(step9Idx !== -1, 'Step 9 not found');
|
||||
const step8Block = src.slice(step8Idx, step9Idx);
|
||||
assert.ok(
|
||||
step8Block.includes('Edit tool') && step8Block.includes('NEVER use Write'),
|
||||
'Step 8 agent prompt missing Edit-only tool discipline',
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 8 references the wait instruction', () => {
|
||||
const step8Idx = src.indexOf('## 8. Spawn gsd-domain-researcher');
|
||||
const step9Idx = src.indexOf('## 9. Spawn gsd-eval-planner');
|
||||
const step8Block = src.slice(step8Idx, step9Idx);
|
||||
assert.ok(
|
||||
step8Block.includes('Wait') || step8Block.includes('wait') || step8Block.includes('complete'),
|
||||
'Step 8 does not instruct orchestrator to wait for Step 7',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user