Merge pull request #1117 from trek-e/fix/parallel-commit-no-verify-1116
This commit is contained in:
@@ -247,6 +247,14 @@ Each executor gets:
|
||||
- Project context (PROJECT.md, STATE.md)
|
||||
- Phase context (CONTEXT.md, RESEARCH.md if available)
|
||||
|
||||
#### Parallel Commit Safety
|
||||
|
||||
When multiple executors run within the same wave, two mechanisms prevent conflicts:
|
||||
|
||||
1. **`--no-verify` commits** — Parallel agents skip pre-commit hooks (which can cause build lock contention, e.g., cargo lock fights in Rust projects). The orchestrator runs `git hook run pre-commit` once after each wave completes.
|
||||
|
||||
2. **STATE.md file locking** — All `writeStateMd()` calls use lockfile-based mutual exclusion (`STATE.md.lock` with `O_EXCL` atomic creation). This prevents the read-modify-write race condition where two agents read STATE.md, modify different fields, and the last writer overwrites the other's changes. Includes stale lock detection (10s timeout) and spin-wait with jitter.
|
||||
|
||||
---
|
||||
|
||||
## Data Flow
|
||||
|
||||
@@ -331,7 +331,10 @@ node gsd-tools.cjs progress [json|table|bar]
|
||||
node gsd-tools.cjs todo complete <filename>
|
||||
|
||||
# Git commit with config checks
|
||||
node gsd-tools.cjs commit <message> [--files f1 f2] [--amend]
|
||||
node gsd-tools.cjs commit <message> [--files f1 f2] [--amend] [--no-verify]
|
||||
```
|
||||
|
||||
> **`--no-verify`**: Skips pre-commit hooks. Used by parallel executor agents during wave-based execution to avoid build lock contention (e.g., cargo lock fights in Rust projects). The orchestrator runs hooks once after each wave completes. Do not use `--no-verify` during sequential execution — let hooks run normally.
|
||||
|
||||
# Web search (requires Brave API key)
|
||||
node gsd-tools.cjs websearch <query> [--limit N] [--freshness day|week|month]
|
||||
|
||||
@@ -133,6 +133,8 @@ To keep planning artifacts out of git:
|
||||
| `parallelization.max_concurrent_agents` | number | `3` | Maximum simultaneous agents |
|
||||
| `parallelization.min_plans_for_parallel` | number | `2` | Minimum plans to trigger parallel execution |
|
||||
|
||||
> **Pre-commit hooks and parallel execution**: When parallelization is enabled, executor agents commit with `--no-verify` to avoid build lock contention (e.g., cargo lock fights in Rust projects). The orchestrator validates hooks once after each wave completes. STATE.md writes are protected by file-level locking to prevent concurrent write corruption. If you need hooks to run per-commit, set `parallelization.enabled: false`.
|
||||
|
||||
---
|
||||
|
||||
## Git Branching
|
||||
|
||||
@@ -245,9 +245,14 @@
|
||||
- Reads PLAN.md with full task instructions
|
||||
- Has access to PROJECT.md, STATE.md, CONTEXT.md, RESEARCH.md
|
||||
- Commits each task atomically with structured commit messages
|
||||
- Uses `--no-verify` on commits during parallel execution to avoid build lock contention
|
||||
- Handles checkpoint types: `auto`, `checkpoint:human-verify`, `checkpoint:decision`, `checkpoint:human-action`
|
||||
- Reports deviations from plan in SUMMARY.md
|
||||
|
||||
**Parallel Safety:**
|
||||
- **Pre-commit hooks**: Skipped by parallel agents (`--no-verify`), run once by orchestrator after each wave
|
||||
- **STATE.md locking**: File-level lockfile prevents concurrent write corruption across agents
|
||||
|
||||
---
|
||||
|
||||
### 6. Work Verification
|
||||
|
||||
@@ -564,6 +564,21 @@ Since v1.17, the installer backs up locally modified files to `gsd-local-patches
|
||||
|
||||
A known workaround exists for a Claude Code classification bug. GSD's orchestrators (execute-phase, quick) spot-check actual output before reporting failure. If you see a failure message but commits were made, check `git log` -- the work may have succeeded.
|
||||
|
||||
### Parallel Execution Causes Build Lock Errors
|
||||
|
||||
If you see pre-commit hook failures, cargo lock contention, or 30+ minute execution times during parallel wave execution, this is caused by multiple agents triggering build tools simultaneously. GSD handles this automatically since v1.26 — parallel agents use `--no-verify` on commits and the orchestrator runs hooks once after each wave. If you're on an older version, add this to your project's `CLAUDE.md`:
|
||||
|
||||
```markdown
|
||||
## Git Commit Rules for Agents
|
||||
All subagent/executor commits MUST use `--no-verify`.
|
||||
```
|
||||
|
||||
To disable parallel execution entirely: `/gsd:settings` → set `parallelization.enabled` to `false`.
|
||||
|
||||
### Windows: Installation Crashes on Protected Directories
|
||||
|
||||
If the installer crashes with `EPERM: operation not permitted, scandir` on Windows, this is caused by OS-protected directories (e.g., Chromium browser profiles). Fixed since v1.24 — update to the latest version. As a workaround, temporarily rename the problematic directory before running the installer.
|
||||
|
||||
---
|
||||
|
||||
## Recovery Quick Reference
|
||||
@@ -581,6 +596,7 @@ A known workaround exists for a Claude Code classification bug. GSD's orchestrat
|
||||
| Update broke local changes | `/gsd:reapply-patches` |
|
||||
| Want session summary for stakeholder | `/gsd:session-report` |
|
||||
| Don't know what step is next | `/gsd:next` |
|
||||
| Parallel execution build errors | Update GSD or set `parallelization.enabled: false` |
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
* state signal-resume Remove WAITING.json signal
|
||||
* resolve-model <agent-type> Get model for agent based on profile
|
||||
* find-phase <phase> Find phase directory by number
|
||||
* commit <message> [--files f1 f2] Commit planning docs
|
||||
* commit <message> [--files f1 f2] [--no-verify] Commit planning docs
|
||||
* verify-summary <path> Verify a SUMMARY.md file
|
||||
* generate-slug <text> Convert text to URL-safe slug
|
||||
* current-timestamp [format] Get timestamp (full|date|filename)
|
||||
@@ -290,6 +290,7 @@ async function main() {
|
||||
|
||||
case 'commit': {
|
||||
const amend = args.includes('--amend');
|
||||
const noVerify = args.includes('--no-verify');
|
||||
const filesIndex = args.indexOf('--files');
|
||||
// Collect all positional args between command name and first flag,
|
||||
// then join them — handles both quoted ("multi word msg") and
|
||||
@@ -298,7 +299,7 @@ async function main() {
|
||||
const messageArgs = args.slice(1, endIndex).filter(a => !a.startsWith('--'));
|
||||
const message = messageArgs.join(' ') || undefined;
|
||||
const files = filesIndex !== -1 ? args.slice(filesIndex + 1).filter(a => !a.startsWith('--')) : [];
|
||||
commands.cmdCommit(cwd, message, files, raw, amend);
|
||||
commands.cmdCommit(cwd, message, files, raw, amend, noVerify);
|
||||
break;
|
||||
}
|
||||
|
||||
|
||||
@@ -214,7 +214,7 @@ function cmdResolveModel(cwd, agentType, raw) {
|
||||
output(result, raw, model);
|
||||
}
|
||||
|
||||
function cmdCommit(cwd, message, files, raw, amend) {
|
||||
function cmdCommit(cwd, message, files, raw, amend, noVerify) {
|
||||
if (!message && !amend) {
|
||||
error('commit message required');
|
||||
}
|
||||
@@ -241,8 +241,9 @@ function cmdCommit(cwd, message, files, raw, amend) {
|
||||
execGit(cwd, ['add', file]);
|
||||
}
|
||||
|
||||
// Commit
|
||||
// Commit (--no-verify skips pre-commit hooks, used by parallel executor agents)
|
||||
const commitArgs = amend ? ['commit', '--amend', '--no-edit'] : ['commit', '-m', message];
|
||||
if (noVerify) commitArgs.push('--no-verify');
|
||||
const commitResult = execGit(cwd, commitArgs);
|
||||
if (commitResult.exitCode !== 0) {
|
||||
if (commitResult.stdout.includes('nothing to commit') || commitResult.stderr.includes('nothing to commit')) {
|
||||
|
||||
@@ -671,10 +671,54 @@ function syncStateFrontmatter(content, cwd) {
|
||||
/**
|
||||
* Write STATE.md with synchronized YAML frontmatter.
|
||||
* All STATE.md writes should use this instead of raw writeFileSync.
|
||||
* Uses a simple lockfile to prevent parallel agents from overwriting
|
||||
* each other's changes (race condition with read-modify-write cycle).
|
||||
*/
|
||||
function writeStateMd(statePath, content, cwd) {
|
||||
const synced = syncStateFrontmatter(content, cwd);
|
||||
fs.writeFileSync(statePath, normalizeMd(synced), 'utf-8');
|
||||
const lockPath = statePath + '.lock';
|
||||
const maxRetries = 10;
|
||||
const retryDelay = 200; // ms
|
||||
|
||||
// Acquire lock (spin with backoff)
|
||||
for (let i = 0; i < maxRetries; i++) {
|
||||
try {
|
||||
// O_EXCL fails if file already exists — atomic lock
|
||||
const fd = fs.openSync(lockPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY);
|
||||
fs.writeSync(fd, String(process.pid));
|
||||
fs.closeSync(fd);
|
||||
break;
|
||||
} catch (err) {
|
||||
if (err.code === 'EEXIST') {
|
||||
// Check for stale lock (> 10s old)
|
||||
try {
|
||||
const stat = fs.statSync(lockPath);
|
||||
if (Date.now() - stat.mtimeMs > 10000) {
|
||||
fs.unlinkSync(lockPath);
|
||||
continue; // retry immediately after clearing stale lock
|
||||
}
|
||||
} catch { /* lock was released between check — retry */ }
|
||||
|
||||
if (i === maxRetries - 1) {
|
||||
// Last resort: write anyway rather than losing data
|
||||
try { fs.unlinkSync(lockPath); } catch {}
|
||||
break;
|
||||
}
|
||||
// Spin-wait with small jitter
|
||||
const jitter = Math.floor(Math.random() * 50);
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < retryDelay + jitter) { /* busy wait */ }
|
||||
continue;
|
||||
}
|
||||
break; // non-EEXIST error — proceed without lock
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
fs.writeFileSync(statePath, normalizeMd(synced), 'utf-8');
|
||||
} finally {
|
||||
try { fs.unlinkSync(lockPath); } catch { /* lock already gone */ }
|
||||
}
|
||||
}
|
||||
|
||||
function cmdStateJson(cwd, raw) {
|
||||
|
||||
@@ -61,6 +61,10 @@ node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" commit "docs: initialize [p
|
||||
|
||||
Each task gets its own commit immediately after completion.
|
||||
|
||||
> **Parallel agents:** When running as a parallel executor (spawned by execute-phase),
|
||||
> use `--no-verify` on all commits to avoid pre-commit hook lock contention.
|
||||
> The orchestrator validates hooks once after all agents complete.
|
||||
|
||||
```
|
||||
{type}({phase}-{plan}): {task-name}
|
||||
|
||||
|
||||
@@ -205,6 +205,14 @@ Execute each wave in sequence. Within a wave: parallel if `PARALLELIZATION=true`
|
||||
Commit each task atomically. Create SUMMARY.md. Update STATE.md and ROADMAP.md.
|
||||
</objective>
|
||||
|
||||
<parallel_execution>
|
||||
You are running as a PARALLEL executor agent. Use --no-verify on all git
|
||||
commits to avoid pre-commit hook contention with other agents. The
|
||||
orchestrator validates hooks once after all agents complete.
|
||||
For gsd-tools commits: add --no-verify flag.
|
||||
For direct git commits: use git commit --no-verify -m "..."
|
||||
</parallel_execution>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/get-shit-done/workflows/execute-plan.md
|
||||
@~/.claude/get-shit-done/templates/summary.md
|
||||
@@ -242,7 +250,17 @@ Execute each wave in sequence. Within a wave: parallel if `PARALLELIZATION=true`
|
||||
|
||||
3. **Wait for all agents in wave to complete.**
|
||||
|
||||
4. **Report completion — spot-check claims first:**
|
||||
4. **Post-wave hook validation (parallel mode only):**
|
||||
|
||||
When agents committed with `--no-verify`, run pre-commit hooks once after the wave:
|
||||
```bash
|
||||
# Run project's pre-commit hooks on the current state
|
||||
git diff --cached --quiet || git stash # stash any unstaged changes
|
||||
git hook run pre-commit 2>&1 || echo "⚠ Pre-commit hooks failed — review before continuing"
|
||||
```
|
||||
If hooks fail: report the failure and ask "Fix hook issues now?" or "Continue to next wave?"
|
||||
|
||||
5. **Report completion — spot-check claims first:**
|
||||
|
||||
For each SUMMARY.md:
|
||||
- Verify first 2 files from `key-files.created` exist on disk
|
||||
|
||||
@@ -234,6 +234,10 @@ See `~/.claude/get-shit-done/references/tdd.md` for structure.
|
||||
|
||||
Your commits may trigger pre-commit hooks. Auto-fix hooks handle themselves transparently — files get fixed and re-staged automatically.
|
||||
|
||||
**If running as a parallel executor agent (spawned by execute-phase):**
|
||||
Use `--no-verify` on all commits. Pre-commit hooks cause build lock contention when multiple agents commit simultaneously (e.g., cargo lock fights in Rust projects). The orchestrator validates once after all agents complete.
|
||||
|
||||
**If running as the sole executor (sequential mode):**
|
||||
If a commit is BLOCKED by a hook:
|
||||
|
||||
1. The `git commit` command fails with hook error output
|
||||
@@ -241,9 +245,7 @@ If a commit is BLOCKED by a hook:
|
||||
3. Fix the issue (type error, lint violation, secret leak, etc.)
|
||||
4. `git add` the fixed files
|
||||
5. Retry the commit
|
||||
6. Do NOT use `--no-verify`
|
||||
|
||||
This is normal and expected. Budget 1-2 retry cycles per commit.
|
||||
6. Budget 1-2 retry cycles per commit
|
||||
</precommit_failure_handling>
|
||||
|
||||
<task_commit>
|
||||
|
||||
Reference in New Issue
Block a user