mirror of
https://github.com/glittercowboy/get-shit-done
synced 2026-04-25 17:25:23 +02:00
fix(#2432,#2424): pre-dispatch PLAN.md commit + reapply-patches baseline detection; docs(#2397): config schema drift (#2469)
- quick.md Step 5.6: commit PLAN.md to base branch before worktree executor spawn when USE_WORKTREES is active, preventing CC #36182 path-resolution drift that caused silent writes to main repo instead of worktree - reapply-patches.md Option A: replace first-add commit heuristic with pristine_hashes SHA-256 matching from backup-meta.json so baseline detection works correctly on multi-cycle repos; first-add fallback kept for older installers without pristine_hashes - CONFIGURATION.md: move security_enforcement/security_asvs_level/security_block_on to workflow.* (matches templates/config.json and workflow readers); rename context_profile → context (matches VALID_CONFIG_KEYS in config.cjs); add planning.sub_repos to schema example - universal-anti-patterns.md + context-budget.md: fix context_window_tokens → context_window (the actual key name in config.cjs) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -129,7 +129,7 @@ The quality of the merge depends on having a **pristine baseline** — the origi
|
||||
|
||||
Check for baseline sources in priority order:
|
||||
|
||||
### Option A: Git history (most reliable)
|
||||
### Option A: Pristine hash from backup-meta.json + git history (most reliable)
|
||||
If the config directory is a git repository:
|
||||
```bash
|
||||
CONFIG_DIR=$(dirname "$PATCHES_DIR")
|
||||
@@ -137,15 +137,35 @@ if git -C "$CONFIG_DIR" rev-parse --git-dir >/dev/null 2>&1; then
|
||||
HAS_GIT=true
|
||||
fi
|
||||
```
|
||||
When `HAS_GIT=true`, use `git log` to find the commit where GSD was originally installed (before user edits). For each file, the pristine baseline can be extracted with:
|
||||
When `HAS_GIT=true`, use the `pristine_hashes` recorded in `backup-meta.json` to locate the correct baseline commit. For each file, iterate commits that touched it and find the one whose blob SHA-256 matches the recorded pristine hash:
|
||||
```bash
|
||||
git -C "$CONFIG_DIR" log --diff-filter=A --format="%H" -- "{file_path}"
|
||||
# Get the expected pristine SHA-256 from backup-meta.json
|
||||
PRISTINE_HASH=$(jq -r ".pristine_hashes[\"${file_path}\"] // empty" "$PATCHES_DIR/backup-meta.json")
|
||||
|
||||
BASELINE_COMMIT=""
|
||||
if [ -n "$PRISTINE_HASH" ]; then
|
||||
# Walk commits that touched this file, pick the one matching the pristine hash
|
||||
while IFS= read -r commit_hash; do
|
||||
blob_hash=$(git -C "$CONFIG_DIR" show "${commit_hash}:${file_path}" 2>/dev/null | sha256sum | cut -d' ' -f1)
|
||||
if [ "$blob_hash" = "$PRISTINE_HASH" ]; then
|
||||
BASELINE_COMMIT="$commit_hash"
|
||||
break
|
||||
fi
|
||||
done < <(git -C "$CONFIG_DIR" log --format="%H" -- "${file_path}")
|
||||
fi
|
||||
|
||||
# Fallback: if no pristine hash in backup-meta (older installer), use first-add commit
|
||||
if [ -z "$BASELINE_COMMIT" ]; then
|
||||
BASELINE_COMMIT=$(git -C "$CONFIG_DIR" log --diff-filter=A --format="%H" -- "${file_path}" | tail -1)
|
||||
fi
|
||||
```
|
||||
This gives the commit that first added the file (the install commit). Extract the pristine version:
|
||||
Extract the pristine version from the matched commit:
|
||||
```bash
|
||||
git -C "$CONFIG_DIR" show {install_commit}:{file_path}
|
||||
git -C "$CONFIG_DIR" show "${BASELINE_COMMIT}:${file_path}"
|
||||
```
|
||||
|
||||
**Why this matters:** `git log --diff-filter=A` returns the commit that *first added* the file, which is the wrong baseline on repos that have been through multiple GSD update cycles. The `pristine_hashes` field in `backup-meta.json` records the SHA-256 of the file as it existed in the pre-update GSD release — matching against it finds the correct baseline regardless of how many updates have occurred.
|
||||
|
||||
### Option B: Pristine snapshot directory
|
||||
Check if a `gsd-pristine/` directory exists alongside `gsd-local-patches/`:
|
||||
```bash
|
||||
|
||||
@@ -21,7 +21,7 @@ GSD stores project settings in `.planning/config.json`. Created during `/gsd-new
|
||||
"search_gitignored": false,
|
||||
"sub_repos": []
|
||||
},
|
||||
"context_profile": null,
|
||||
"context": null,
|
||||
"workflow": {
|
||||
"research": true,
|
||||
"plan_check": true,
|
||||
@@ -394,6 +394,8 @@ Control confirmation prompts during workflows.
|
||||
|
||||
Settings for the security enforcement feature (v1.31). All follow the **absent = enabled** pattern. These keys live under `workflow.*` in `.planning/config.json` — matching the shipped template and the runtime reads in `workflows/plan-phase.md`, `workflows/execute-phase.md`, `workflows/secure-phase.md`, and `workflows/verify-work.md`.
|
||||
|
||||
These keys live under `workflow.*` — that is where the workflows and installer write and read them. Setting them at the top level of `config.json` is silently ignored.
|
||||
|
||||
| Setting | Type | Default | Description |
|
||||
|---------|------|---------|-------------|
|
||||
| `workflow.security_enforcement` | boolean | `true` | Enable threat-model-anchored security verification via `/gsd-secure-phase`. When `false`, security checks are skipped entirely |
|
||||
|
||||
@@ -12,7 +12,7 @@ Every workflow that spawns agents or reads significant content must follow these
|
||||
|
||||
1. **Never** read agent definition files (`agents/*.md`) -- `subagent_type` auto-loads them
|
||||
2. **Never** inline large files into subagent prompts -- tell agents to read files from disk instead
|
||||
3. **Read depth scales with context window** -- check `context_window_tokens` in `.planning/config.json`:
|
||||
3. **Read depth scales with context window** -- check `context_window` in `.planning/config.json`:
|
||||
- At < 500000 tokens (default 200k): read only frontmatter, status fields, or summaries. Never read full SUMMARY.md, VERIFICATION.md, or RESEARCH.md bodies.
|
||||
- At >= 500000 tokens (1M model): MAY read full subagent output bodies when the content is needed for inline presentation or decision-making. Still avoid unnecessary reads.
|
||||
4. **Delegate** heavy work to subagents -- the orchestrator routes, it doesn't execute
|
||||
@@ -25,7 +25,7 @@ Every workflow that spawns agents or reads significant content must follow these
|
||||
| < 500k (200k model) | Frontmatter only | Frontmatter only | Frontmatter only | Current phase only |
|
||||
| >= 500k (1M model) | Full body permitted | Full body permitted | Full body permitted | Current phase only |
|
||||
|
||||
**How to check:** Read `.planning/config.json` and inspect `context_window_tokens`. If the field is absent, treat as 200k (conservative default).
|
||||
**How to check:** Read `.planning/config.json` and inspect `context_window`. If the field is absent, treat as 200k (conservative default).
|
||||
|
||||
## Context Degradation Tiers
|
||||
|
||||
|
||||
@@ -8,13 +8,13 @@ Rules that apply to ALL workflows and agents. Individual workflows may have addi
|
||||
|
||||
1. **Never** read agent definition files (`agents/*.md`) -- `subagent_type` auto-loads them. Reading agent definitions into the orchestrator wastes context for content automatically injected into subagent sessions.
|
||||
2. **Never** inline large files into subagent prompts -- tell agents to read files from disk instead. Agents have their own context windows.
|
||||
3. **Read depth scales with context window** -- check `context_window_tokens` in `.planning/config.json`. At < 500000: read only frontmatter, status fields, or summaries. At >= 500000 (1M model): full body reads permitted when content is needed for inline decisions. See `references/context-budget.md` for the complete table.
|
||||
3. **Read depth scales with context window** -- check `context_window` in `.planning/config.json`. At < 500000: read only frontmatter, status fields, or summaries. At >= 500000 (1M model): full body reads permitted when content is needed for inline decisions. See `references/context-budget.md` for the complete table.
|
||||
4. **Delegate** heavy work to subagents -- the orchestrator routes, it does not build, analyze, research, investigate, or verify.
|
||||
5. **Proactive pause warning**: If you have already consumed significant context (large file reads, multiple subagent results), warn the user: "Context budget is getting heavy. Consider checkpointing progress."
|
||||
|
||||
## File Reading Rules
|
||||
|
||||
6. **SUMMARY.md read depth scales with context window** -- at context_window_tokens < 500000: read frontmatter only from prior phase SUMMARYs. At >= 500000: full body reads permitted for direct-dependency phases. Transitive dependencies (2+ phases back) remain frontmatter-only regardless.
|
||||
6. **SUMMARY.md read depth scales with context window** -- at context_window < 500000: read frontmatter only from prior phase SUMMARYs. At >= 500000: full body reads permitted for direct-dependency phases. Transitive dependencies (2+ phases back) remain frontmatter-only regardless.
|
||||
7. **Never** read full PLAN.md files from other phases -- only current phase plans.
|
||||
8. **Never** read `.planning/logs/` files -- only the health workflow reads these.
|
||||
9. **Do not** re-read full file contents when frontmatter is sufficient -- frontmatter contains status, key_files, commits, and provides fields. Exception: at >= 500000, re-reading full body is acceptable when semantic content is needed.
|
||||
|
||||
@@ -567,6 +567,24 @@ Offer: 1) Force proceed, 2) Abort
|
||||
|
||||
---
|
||||
|
||||
**Step 5.6: Pre-dispatch plan commit (worktree mode only)**
|
||||
|
||||
When `USE_WORKTREES !== "false"`, commit PLAN.md to the current branch **before** spawning the executor. This ensures the worktree inherits PLAN.md at its branch HEAD so the executor can read it via a worktree-rooted path — avoiding the main-repo path priming that triggers CC #36182 path-resolution drift.
|
||||
|
||||
Skip this step entirely if `USE_WORKTREES === "false"` (non-worktree mode: PLAN.md is committed in Step 8 as usual).
|
||||
|
||||
```bash
|
||||
if [ "${USE_WORKTREES}" != "false" ]; then
|
||||
COMMIT_DOCS=$(gsd-sdk query config-get commit_docs 2>/dev/null || echo "true")
|
||||
if [ "$COMMIT_DOCS" != "false" ]; then
|
||||
git add "${QUICK_DIR}/${quick_id}-PLAN.md"
|
||||
git commit --no-verify -m "docs(${quick_id}): pre-dispatch plan for ${DESCRIPTION}" -- "${QUICK_DIR}/${quick_id}-PLAN.md" || true
|
||||
fi
|
||||
fi
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
**Step 6: Spawn executor**
|
||||
|
||||
Capture current HEAD before spawning (used for worktree branch check):
|
||||
|
||||
80
tests/bug-2424-reapply-patches-baseline-detection.test.cjs
Normal file
80
tests/bug-2424-reapply-patches-baseline-detection.test.cjs
Normal file
@@ -0,0 +1,80 @@
|
||||
/**
|
||||
* Bug #2424: reapply-patches pristine-baseline detection uses first-add commit
|
||||
*
|
||||
* The three-way merge baseline detection previously used `git log --diff-filter=A`
|
||||
* which returns the commit that FIRST added the file. On repos that have been
|
||||
* through multiple GSD update cycles, this returns a stale, many-versions-old
|
||||
* baseline — not the version immediately prior to the current update.
|
||||
*
|
||||
* Fix: Option A must prefer `pristine_hashes` from backup-meta.json to locate
|
||||
* the correct baseline commit by SHA-256 matching, with a fallback to the
|
||||
* first-add heuristic only when no pristine hash is recorded.
|
||||
*/
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const REAPPLY_MD = path.join(__dirname, '..', 'commands', 'gsd', 'reapply-patches.md');
|
||||
|
||||
describe('reapply-patches pristine baseline detection (#2424)', () => {
|
||||
let content;
|
||||
|
||||
test('reapply-patches.md exists', () => {
|
||||
assert.ok(fs.existsSync(REAPPLY_MD), 'commands/gsd/reapply-patches.md must exist');
|
||||
content = fs.readFileSync(REAPPLY_MD, 'utf-8');
|
||||
});
|
||||
|
||||
test('Option A references pristine_hashes from backup-meta.json', () => {
|
||||
const optionAStart = content.indexOf('### Option A');
|
||||
const optionBStart = content.indexOf('### Option B');
|
||||
assert.ok(optionAStart !== -1, 'Option A section must exist');
|
||||
assert.ok(optionBStart !== -1, 'Option B section must exist');
|
||||
const optionABlock = content.slice(optionAStart, optionBStart);
|
||||
assert.ok(
|
||||
optionABlock.includes('pristine_hashes'),
|
||||
'Option A must use pristine_hashes from backup-meta.json as the primary baseline source'
|
||||
);
|
||||
assert.ok(
|
||||
optionABlock.includes('backup-meta.json'),
|
||||
'Option A must explicitly read backup-meta.json for the pristine hash'
|
||||
);
|
||||
});
|
||||
|
||||
test('Option A iterates commit history to find hash-matching commit', () => {
|
||||
const optionAStart = content.indexOf('### Option A');
|
||||
const optionBStart = content.indexOf('### Option B');
|
||||
const optionABlock = content.slice(optionAStart, optionBStart);
|
||||
// Must walk commits and compare hashes — not just take the first-add commit
|
||||
assert.ok(
|
||||
optionABlock.includes('sha256') || optionABlock.includes('SHA-256') || optionABlock.includes('sha256sum'),
|
||||
'Option A must compare SHA-256 hashes to identify the correct baseline commit'
|
||||
);
|
||||
assert.ok(
|
||||
optionABlock.includes('git log') && optionABlock.includes('format="%H"'),
|
||||
'Option A must iterate git log commits to find the hash-matching baseline'
|
||||
);
|
||||
});
|
||||
|
||||
test('Option A has a fallback to first-add heuristic when no pristine hash is available', () => {
|
||||
const optionAStart = content.indexOf('### Option A');
|
||||
const optionBStart = content.indexOf('### Option B');
|
||||
const optionABlock = content.slice(optionAStart, optionBStart);
|
||||
assert.ok(
|
||||
optionABlock.includes('diff-filter=A') || optionABlock.includes('Fallback') || optionABlock.includes('fallback'),
|
||||
'Option A must include a fallback for repos without pristine_hashes (older installer)'
|
||||
);
|
||||
});
|
||||
|
||||
test('Option A explains why first-add commit is wrong for multi-cycle repos', () => {
|
||||
const optionAStart = content.indexOf('### Option A');
|
||||
const optionBStart = content.indexOf('### Option B');
|
||||
const optionABlock = content.slice(optionAStart, optionBStart);
|
||||
assert.ok(
|
||||
optionABlock.includes('first add') || optionABlock.includes('first added') ||
|
||||
optionABlock.includes('multiple') || optionABlock.includes('update cycles'),
|
||||
'Option A must document why the first-add heuristic fails for multi-cycle repos'
|
||||
);
|
||||
});
|
||||
});
|
||||
102
tests/bug-2432-quick-plan-predispatch-commit.test.cjs
Normal file
102
tests/bug-2432-quick-plan-predispatch-commit.test.cjs
Normal file
@@ -0,0 +1,102 @@
|
||||
/**
|
||||
* Bug #2432: quick.md PLAN.md timing — worktree executor can't read PLAN.md
|
||||
*
|
||||
* The orchestrator must commit PLAN.md to the base branch BEFORE spawning the
|
||||
* worktree-isolated executor. Without this, the executor's first Read resolves
|
||||
* to a main-repo absolute path (not a worktree path), which primes CC's path
|
||||
* cache and causes subsequent Edit/Write calls to silently target the main repo
|
||||
* instead of the worktree (CC issue #36182 amplifier).
|
||||
*
|
||||
* Fix: Step 5.6 commits PLAN.md pre-dispatch when USE_WORKTREES is active.
|
||||
*/
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const QUICK_MD = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md');
|
||||
|
||||
describe('quick.md pre-dispatch PLAN.md commit (#2432)', () => {
|
||||
let content;
|
||||
|
||||
test('quick.md exists', () => {
|
||||
assert.ok(fs.existsSync(QUICK_MD), 'get-shit-done/workflows/quick.md must exist');
|
||||
content = fs.readFileSync(QUICK_MD, 'utf-8');
|
||||
});
|
||||
|
||||
test('Step 5.6 exists between Step 5.5 and Step 6', () => {
|
||||
const step55 = content.indexOf('Step 5.5');
|
||||
const step56 = content.indexOf('Step 5.6');
|
||||
const step6 = content.indexOf('Step 6:');
|
||||
assert.ok(step55 !== -1, 'Step 5.5 must exist');
|
||||
assert.ok(step56 !== -1, 'Step 5.6 must exist');
|
||||
assert.ok(step6 !== -1, 'Step 6 must exist');
|
||||
assert.ok(step56 > step55, 'Step 5.6 must appear after Step 5.5');
|
||||
assert.ok(step56 < step6, 'Step 5.6 must appear before Step 6');
|
||||
});
|
||||
|
||||
test('Step 5.6 is gated on USE_WORKTREES', () => {
|
||||
const step56Start = content.indexOf('Step 5.6');
|
||||
const step6Start = content.indexOf('Step 6:', step56Start);
|
||||
const step56Block = content.slice(step56Start, step6Start);
|
||||
assert.ok(
|
||||
step56Block.includes('USE_WORKTREES'),
|
||||
'Step 5.6 must be gated on USE_WORKTREES — only commit pre-dispatch in worktree mode'
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 5.6 is gated on commit_docs', () => {
|
||||
const step56Start = content.indexOf('Step 5.6');
|
||||
const step6Start = content.indexOf('Step 6:', step56Start);
|
||||
const step56Block = content.slice(step56Start, step6Start);
|
||||
assert.ok(
|
||||
step56Block.includes('commit_docs'),
|
||||
'Step 5.6 must respect commit_docs config — skip pre-dispatch commit when commit_docs is false'
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 5.6 stages and commits PLAN.md', () => {
|
||||
const step56Start = content.indexOf('Step 5.6');
|
||||
const step6Start = content.indexOf('Step 6:', step56Start);
|
||||
const step56Block = content.slice(step56Start, step6Start);
|
||||
assert.ok(
|
||||
step56Block.includes('PLAN.md'),
|
||||
'Step 5.6 must reference PLAN.md in the pre-dispatch commit'
|
||||
);
|
||||
assert.ok(
|
||||
step56Block.includes('git add') || step56Block.includes('git commit'),
|
||||
'Step 5.6 must include git add/commit to stage and commit PLAN.md'
|
||||
);
|
||||
});
|
||||
|
||||
test('Step 5.6 uses --no-verify to avoid hook interference', () => {
|
||||
const step56Start = content.indexOf('Step 5.6');
|
||||
const step6Start = content.indexOf('Step 6:', step56Start);
|
||||
const step56Block = content.slice(step56Start, step6Start);
|
||||
assert.ok(
|
||||
step56Block.includes('--no-verify'),
|
||||
'Step 5.6 pre-dispatch commit must use --no-verify to avoid hook interference'
|
||||
);
|
||||
});
|
||||
|
||||
test('executor prompt references PLAN.md via relative (worktree-rooted) path', () => {
|
||||
// QUICK_DIR is always set to ".planning/quick/..." (relative) so ${QUICK_DIR}/...PLAN.md
|
||||
// resolves relative to the worktree root, not the main repo absolute path.
|
||||
// Verify the executor prompt uses QUICK_DIR variable (not a hardcoded absolute path).
|
||||
const executorTask = content.indexOf('subagent_type="gsd-executor"');
|
||||
assert.ok(executorTask !== -1, 'executor Task() spawn must exist');
|
||||
// Find the files_to_read block near the executor spawn
|
||||
const filesBlock = content.lastIndexOf('<files_to_read>', executorTask);
|
||||
const filesBlockEnd = content.indexOf('</files_to_read>', filesBlock);
|
||||
const filesContent = content.slice(filesBlock, filesBlockEnd);
|
||||
assert.ok(
|
||||
filesContent.includes('QUICK_DIR') || filesContent.includes('.planning/quick'),
|
||||
'executor files_to_read must reference PLAN.md via relative QUICK_DIR path'
|
||||
);
|
||||
assert.ok(
|
||||
!filesContent.match(/\/home\/|\/Users\/|\/root\//),
|
||||
'executor files_to_read must NOT contain hardcoded absolute paths'
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user