fix(3678): executor must respect commit_docs:false; teach SDK skip envelope by trek-e · Pull Request #3679 · gsd-build/get-shit-done

trek-e · 2026-05-17T21:15:35Z

Fix PR

Linked Issue

Issue carries both confirmed (canonical) and confirmed-bug (template-required) labels.

What was broken

When a user set commit_docs: false in .planning/config.json, gsd-executor was still emitting docs({phase}-{plan}): complete plan summary commits that force-staged gitignored .planning/ artifacts into the user's git history. The reporter (#3678) saw a 🔧 chore: stop tracking gitignored build artifacts commit appear shortly after — the agent self-correcting after it noticed the leak.

What this fix does

Teaches agents/gsd-executor.md how to interpret the SDK's {committed:false, skipped:true, ...} return envelope, adds skipped: true as a first-class field on that envelope in get-shit-done/bin/lib/commands.cjs::cmdCommit, and bans git add -f / git add --force from any agent or workflow body via a regression test.

Root cause

The SDK guard at get-shit-done/bin/lib/commands.cjs:268-280 correctly skips when commit_docs:false and returns {committed: false, hash: null, reason: 'skipped_commit_docs_false'} — no git ops happen.

But the executor agent prompt at agents/gsd-executor.md:710-720 (the <final_commit> block) just instructs the LLM to call gsd-sdk query commit "docs(...)" --files .planning/... with no instruction for the skipped return. With no explicit handling, the LLM improvises: "the SDK didn't make the commit I was told to make, so I'll do it manually with git add -f / git commit". That bypasses the gitignore and lands the regression.

The only place commit_docs appeared in the entire executor agent prompt before this PR was line 80, listing it as a JSON field to extract. Zero behavioral instructions tied to it.

Testing

How I verified the fix

node --test tests/bug-3678-executor-commit-docs-respect.test.cjs — 7/7 pass. The test runs RED before either fix (3 failures across A/B), GREEN after applying the agent-prompt update AND the SDK envelope change, then I tightened a single prose line so the prohibition-sentence exception in C1 wouldn't trip on the new audit text.

Regression test added?

Yes — tests/bug-3678-executor-commit-docs-respect.test.cjs (230 LOC, 7 tests across 3 describe groups). Carries // allow-test-rule: source-text-is-the-product for the prompt/workflow body inspection.

Coverage breakdown:

Test	Asserts
A1	`agents/gsd-executor.md` references the `skipped` envelope (one of: `'skipped_commit_docs_false'`, `committed: false`, `skipped: true`)
A2	`agents/gsd-executor.md` carries an explicit "do not fall back to raw git" / "must not fall back" / "never run git add" instruction
B1	`gsd-tools commit` with `commit_docs:false` returns `{committed:false, skipped:true, reason:'skipped_commit_docs_false'}` (frozen enum)
B2	`git diff --cached --name-only` shows no `.planning/` paths after the skipped call
B3	`git rev-parse HEAD` is unchanged after the skipped call
C1	No agent body contains `git add -f` / `git add --force` outside a prohibition or audit sentence (regex windowed)
C2	Same for workflow bodies

Platforms tested

macOS (local node --test)
Linux (docker via gsd-test-summary, host cartographer)
Windows — N/A (no path-handling code touched; only .md prose, a 1-line result field add in JS, and a test that uses node:fs + execFileSync('git', ...))

Runtimes tested

N/A — the bug is in the runtime-agnostic SDK and the LLM-prompt body. Fix applies to every runtime that loads agents/gsd-executor.md.

Scope confirmation

Bug spec is the issue body; root cause matches reporter's hypothesis exactly
No scope creep — no other agent prompts touched, no other SDK verbs changed

Documentation

The agent prompt itself IS the documentation surface for this contract. The new text in agents/gsd-executor.md is the user-facing explanation.
Changeset fragment added below

Checklist

Issue linked above with Fixes #3678
Linked issue has confirmed + confirmed-bug labels
Bug reproduced locally via code trace; SDK guard verified working; agent-prompt gap verified as the cause
All existing tests pass (gsd-test-summary 11751/0)
New tests cover happy path, edge case (skipped envelope), and structural ban (no force-add)
.changeset/ fragment added
No new dependencies
Works on Windows — no path-handling code touched

Breaking changes

None. The SDK envelope change is purely additive: existing callers reading committed / hash / reason continue to work. The new skipped field is opt-in. The agent prompt change adds text only, removing nothing.

Discovery context

Diagnosed via the /diagnose skill discipline:

Phase 1 (feedback loop): code-trace plus structured behavioral test against gsd-tools commit (no LLM-in-the-loop needed — the prompt gap is statically observable).
Phase 2 (reproduce): confirmed SDK guard works; located the unguarded handoff at agents/gsd-executor.md:710-720; confirmed zero commit_docs-related behavioral instructions exist in the prompt body.
Phase 3 (hypothesise): 5 ranked hypotheses; H1 (missing prompt instruction → LLM improvisation) confirmed by code-reading evidence; H3 (git add -f baked into a workflow path) falsified by grep; H4 (SDK uses force-add) falsified by inspection of cmdCommit.
Phase 4-5: TDD red-green-refactor; both source fixes plus regression test landed together.
Phase 6: all [DEBUG-*] instrumentation absent (none added); regression test prevents drift; what-would-have-prevented-this is the C1/C2 structural ban that now exists.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Planning artifacts are no longer force-committed when docs commitment is disabled; the system now records explicit skip reasons for intentional non‑commits and treats them as acceptable completion.
Tests
- Added a regression test verifying planning artifacts respect disabled docs commitment and that no force-staging occurs; added a repository-wide check banning forced staging in agent/workflow content.

…velope Closes #3678 When `commit_docs: false` in `.planning/config.json`, the SDK's `cmdCommit` correctly short-circuits and returns `{committed: false, hash: null, reason: 'skipped_commit_docs_false'}` without staging or committing anything. The agent prompt at `agents/gsd-executor.md:710-720` (final_commit block) tells the executor to call `gsd-sdk query commit "docs(...)" --files .planning/...` but says NOTHING about how to interpret a skipped return. With no explicit instruction, the LLM improvises raw `git add` / `git add -f` / `git commit` to "fulfill" the per-plan commit step it was told to make, which leaks gitignored `.planning/` artifacts into the user's git history (exactly what the reporter observed). Three coordinated fixes: 1. **agents/gsd-executor.md final_commit block** — adds explicit handling text for all three SDK return envelopes (`committed:true`, `skipped:true commit_docs`, `skipped:true gitignored`, `committed:false other reasons`). States plainly: "Do not fall back to raw `git add` / `git commit` / `git add -f` when the SDK returns `skipped: true`." 2. **get-shit-done/bin/lib/commands.cjs cmdCommit** — adds `skipped: true` to both skip-path envelopes so agents see "skipped" as a first-class success signal rather than inferring "no commit happened, I must improvise" from absent `hash` / `committed:false`. Backward-compatible: existing callers reading `committed` / `hash` / `reason` are unaffected. 3. **tests/bug-3678-executor-commit-docs-respect.test.cjs** — 7-test regression covering: - A1/A2: agent prompt mentions the skip envelope AND explicitly forbids raw-git fallback (`source-text-is-the-product` exception) - B1: SDK envelope carries `committed:false`, `skipped:true`, canonical `reason: 'skipped_commit_docs_false'` (frozen enum) - B2: git index empty after commit_docs:false skip (no `.planning/` staged) - B3: HEAD unchanged after commit_docs:false skip - C1/C2: structural ban on `git add -f` / `git add --force` in any agent or workflow body (prohibition-sentence exception preserves audit prose) Verification: - node --test tests/bug-3678-*: 7/7 pass - Targeted regression (10 commit/executor-adjacent files): 135/135 pass - Full docker suite (gsd-test-summary): 11751/11740 pass / 0 fail (the 11 added are this test plus a few collateral pickups) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…Opus 4.7 (1M context) <[email protected]>)

… (1M context) <[email protected]>)

coderabbitai · 2026-05-17T21:15:46Z

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ee2f9024-9684-4bb6-b1d6-5565b517a51a

📥 Commits

Reviewing files that changed from the base of the PR and between 335f5ad and 330d693.

📒 Files selected for processing (2)

agents/gsd-executor.md
tests/bug-3678-executor-commit-docs-respect.test.cjs

 ____________________________________________________________
< If you don't finish then you're just busy, not productive. >
 ------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

📝 Walkthrough

Walkthrough

This PR fixes issue #3678 by making the executor respect commit_docs: false settings. The SDK's cmdCommit now returns explicit skipped: true signals when gating conditions prevent committing, the executor prompt is updated to handle those signals and forbid git fallbacks, and comprehensive regression tests validate the contract and structural guards.

Changes

commit_docs: false respects SDK skip envelope

Layer / File(s)	Summary
SDK skip envelope contract `get-shit-done/bin/lib/commands.cjs`	`cmdCommit` now returns `{ committed: false, skipped: true, reason: 'skipped_commit_docs_false' \| 'skipped_gitignored' }` for commit-gating conditions instead of only `committed: false`.
gsd-executor prompt update `agents/gsd-executor.md`	Documents the SDK envelope shapes and executor's required handling for success and skip paths; explicitly forbids falling back to raw `git add`/`git commit` when the SDK reports `skipped: true`.
Regression test suite `tests/bug-3678-executor-commit-docs-respect.test.cjs`	Validates prompt contract (executor.md text assertions), SDK behavior (commit_docs: false envelope and git state preservation), and structural guards (prohibits `git add -f`/`--force` outside warning sentences).
Change documentation `.changeset/fix-3678-commit-docs-respect.md`	Changeset entry documenting the behavioral change and regression test additions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

#3678: This PR directly fixes the root cause—the executor now treats the SDK's skip envelope as authoritative, and the SDK's cmdCommit now returns explicit skipped: true signals when commit_docs: false.

Suggested labels

bug, area: agents, area: core, size/M, needs-review, confirmed-bug

Suggested reviewers

glittercowboy
Ari4ka

Poem

🐰 The executor now listens true,
To SDK skip signals, shiny and new—
No force-adds past our config's gate,
When commit_docs says wait, we wait!
A rabbit-tested regression suite,
Keeps .planning/ in its place. ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically identifies the main fix: teaching the executor to respect commit_docs:false via the SDK skip envelope.
Description check	✅ Passed	The description is comprehensive, well-structured, and follows the fix template with all required sections: linked issue, root cause analysis, what was fixed, testing details, regression test coverage, and scope confirmation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/3678-commit-docs-force

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@agents/gsd-executor.md`:
- Around line 721-741: Update the executor logic so that responses from "gsd-sdk
query commit" with "skipped: true" (reasons 'skipped_commit_docs_false' or
'skipped_gitignored') are treated as terminal successes and do not trigger any
raw-git fallback (no git add -f, git commit, or forced metadata commits); adjust
the checklist/final-metadata-commit path to consider a prior commit result with
skipped:true as completed (record the appropriate "skipped (...)" completion
message) and skip attempting a subsequent unconditional metadata commit or
force-staging of .planning files.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: eeb72bc1-b669-49b8-9f26-755ecf60352c

📥 Commits

Reviewing files that changed from the base of the PR and between f970c09 and 335f5ad.

📒 Files selected for processing (4)

.changeset/fix-3678-commit-docs-respect.md
agents/gsd-executor.md
get-shit-done/bin/lib/commands.cjs
tests/bug-3678-executor-commit-docs-respect.test.cjs

…list The new `final_commit` prose at lines 717-741 teaches the executor to treat `skipped:true` as success and forbids raw-git fallback, but the downstream completion checklist still contained an unconditional "Final metadata commit made" checkbox. An LLM executor reading an unchecked mandatory box may attempt to satisfy it via raw `git add`, re-introducing the exact regression this PR is meant to prevent. Update the checklist line to carve out the intentional-skip case and add a regression test asserting the carve-out remains present. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

trek-e and others added 2 commits May 17, 2026 17:13

chore(changeset): add fragment for #3678 fix (Co-Authored-By: Claude …

799d91f

…Opus 4.7 (1M context) <[email protected]>)

trek-e requested a review from glittercowboy as a code owner May 17, 2026 21:15

chore(changeset): set PR number 3679 (Co-Authored-By: Claude Opus 4.7…

335f5ad

… (1M context) <[email protected]>)

github-actions Bot added the size/L label May 17, 2026

trek-e mentioned this pull request May 17, 2026

gsd-executor force-commits .planning/ files when commit_docs is false #3678

Open

1 task

coderabbitai Bot reviewed May 17, 2026

View reviewed changes

Comment thread agents/gsd-executor.md

github-actions Bot added size/L and removed size/L labels May 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(3678): executor must respect commit_docs:false; teach SDK skip envelope#3679

fix(3678): executor must respect commit_docs:false; teach SDK skip envelope#3679
trek-e wants to merge 4 commits into
mainfrom
fix/3678-commit-docs-force

trek-e commented May 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 17, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Suggested labels

Suggested reviewers

Poem

coderabbitai Bot left a comment

Uh oh!

Labels

1 participant

Uh oh!

Conversation

trek-e commented May 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix PR

Linked Issue

What was broken

What this fix does

Root cause

Testing

How I verified the fix

Regression test added?

Platforms tested

Runtimes tested

Scope confirmation

Documentation

Checklist

Breaking changes

Discovery context

Summary by CodeRabbit

coderabbitai Bot commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Suggested labels

Suggested reviewers

Poem

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Labels

1 participant

trek-e commented May 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 17, 2026 •

edited

Loading