From 85d7a8b4b174d9ff8d6b042cd0e5f2385e05e803 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 15 May 2026 08:16:26 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(3542):=20prohibit=20git=20stash=20in=20?= =?UTF-8?q?executor=20agents=20=E2=80=94=20shared=20stash=20storage=20viol?= =?UTF-8?q?ates=20worktree=20isolation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #3542 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../3542-executor-git-stash-prohibition.md | 5 + agents/gsd-executor.md | 24 +++ get-shit-done/workflows/execute-phase.md | 2 +- ...42-executor-git-stash-prohibition.test.cjs | 174 ++++++++++++++++++ 4 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 .changeset/3542-executor-git-stash-prohibition.md create mode 100644 tests/bug-3542-executor-git-stash-prohibition.test.cjs diff --git a/.changeset/3542-executor-git-stash-prohibition.md b/.changeset/3542-executor-git-stash-prohibition.md new file mode 100644 index 000000000..6439f5065 --- /dev/null +++ b/.changeset/3542-executor-git-stash-prohibition.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 0 +--- +**Executor agents are now forbidden from running any `git stash` subcommand inside worktree isolation** — git stores stashes at `refs/stash` in the parent `.git/` directory, so the stash list is shared across the main checkout and every linked worktree. A `git stash pop` inside an executor's worktree silently applied WIP pushed from a prior sibling-worktree session, producing UU/UD merge-conflict states and phantom untracked files that violated the `isolation="worktree"` invariant. The `` block in `agents/gsd-executor.md` now lists the full `git stash` family alongside `--no-verify`, `--hard`, and `git update-ref`, with sanctioned alternatives (commit to a throwaway branch you own, or read-only `git show :` / `git diff -- `). The orchestrator-side post-wave-hook helper in `execute-phase.md` continues to use stash deliberately and is now annotated to make the single-checkout precondition explicit. Closes #3542. diff --git a/agents/gsd-executor.md b/agents/gsd-executor.md index 1b918c527..018d01e0f 100644 --- a/agents/gsd-executor.md +++ b/agents/gsd-executor.md @@ -551,6 +551,30 @@ back, those deletions appear on the main branch, destroying prior-wave work (#20 `` and per-commit `` are the correct prevention; if either fails, the workflow MUST stop, not self-heal. - `git push --force` / `git push -f` to any branch you did not create. +- `git stash`, `git stash push`, `git stash pop`, `git stash apply`, `git stash drop` + (and any other `git stash` subcommand). **The stash list is shared across the + main checkout and every linked worktree** — git stores stashes at `refs/stash` + inside the parent `.git/` directory, not inside the per-worktree + `.git/worktrees//` subdirectory. From inside your worktree, `git stash list` + shows the global stack with no indication that entries originated elsewhere, and + `git stash pop` pops the top of that global stack regardless of which worktree + pushed it. Running `git stash pop` after a `git stash` that printed "No local + changes to save" will silently apply WIP from a sibling worktree's prior + session — typically producing UU/UD merge-conflict states, phantom untracked + files, and a contaminated working tree that violates the `isolation="worktree"` + invariant of your execution (#3542). + + **Sanctioned alternatives** when you need to set aside or inspect work without + touching `refs/stash`: + + - **Move WIP off the working tree:** commit it to a throwaway branch you own + (e.g. `git checkout -b scratch-/-wip && git add -A && git commit -m "wip"`), + then `git checkout ` to return to your task. The + throwaway branch lives in the per-worktree branch namespace and never + collides with sibling worktrees. + - **Read-only inspection of another ref:** use `git show :` to + print a file at any ref, or `git diff -- ` to compare. Neither + mutates `refs/stash` nor leaks state across worktrees. If you need to discard changes to a specific file you modified during this task, use: ```bash diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index e6f052064..f95c76301 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -717,7 +717,7 @@ increases monotonically across waves. `{status}` is `complete` (success), ```bash SKIP_HOOKS=$(gsd-sdk query config-get workflow.worktree_skip_hooks 2>/dev/null || echo "false") if [ "$SKIP_HOOKS" = "true" ]; then - # Stash uncommitted changes under a named ref so we always pop (bare `git stash` strands them on hook/script failure). + # Stash uncommitted changes under a named ref so we always pop (bare `git stash` strands them on hook/script failure). #3542: `refs/stash` is shared across worktrees, so this helper runs ONLY in the orchestrator's main checkout after all wave worktrees have been merged + removed; executors are forbidden from running any `git stash` subcommand (see `` in `agents/gsd-executor.md`). STASHED=false if (! git diff --quiet || ! git diff --cached --quiet) && git stash push -u -m "gsd-post-wave-hook-$$" >/dev/null 2>&1; then STASHED=true; fi git hook run pre-commit 2>&1 || echo "⚠ Pre-commit hooks failed — review before continuing" diff --git a/tests/bug-3542-executor-git-stash-prohibition.test.cjs b/tests/bug-3542-executor-git-stash-prohibition.test.cjs new file mode 100644 index 000000000..9406bf262 --- /dev/null +++ b/tests/bug-3542-executor-git-stash-prohibition.test.cjs @@ -0,0 +1,174 @@ +// allow-test-rule: source-text-is-the-product +// Bug #3542 — Worktree stash storage is shared across agent worktrees; +// `git stash pop` from an executor agent contaminates its isolation. +// +// Git stores stashes at `refs/stash` (plus the stash reflog) inside the +// PARENT `.git/` directory. Every linked worktree shares that ref, so a +// `git stash push` in any worktree (or in the main checkout) is visible — +// and poppable — from every other worktree. From inside a worktree, +// `git stash list` shows the shared list with no indication that an entry +// originated elsewhere. +// +// Incident: an executor agent ran `git stash` (printed "No local changes +// to save" — nothing pushed), then `git stash pop`, which yanked a stash +// from a prior worktree-agent session. Result: 21 files in UU/UD state, +// 16 phantom untracked files, ~12 minutes of recovery work. This breaks +// the `isolation="worktree"` invariant documented in the executor agent. +// +// Two test cases: +// +// A. The agent prompt content asserts the `git stash` family is +// prohibited and documents an alternative. The prompt content IS +// the runtime contract for the agent — source-text-is-the-product +// (per CONTEXT.md `RULESET.TESTS.no-source-grep.exemption`). +// +// B. A behavioural test that pins the git invariant the prohibition +// defends against: a stash pushed in the main checkout is visible in +// a linked worktree's `git stash list`, proving stash storage is +// shared and cannot be relied on for worktree-scoped isolation. + +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { execSync } = require('node:child_process'); + +const EXECUTOR_PATH = path.join(__dirname, '..', 'agents', 'gsd-executor.md'); + +// ─── Test A — prompt content asserts the prohibition ─────────────────────── + +test('bug-3542: gsd-executor.md prohibits `git stash` family inside worktrees', () => { + const content = fs.readFileSync(EXECUTOR_PATH, 'utf-8'); + + // The prohibition must call out `git stash` explicitly. Just listing + // "stash" isn't enough — the existing post-wave-hook helper script + // legitimately mentions stash, so we look for the specific forbidden + // commands the agent must never run on its own. + assert.match( + content, + /`git stash`/, + 'gsd-executor.md must explicitly forbid `git stash` (bare push) — see #3542', + ); + assert.match( + content, + /`git stash pop`/, + 'gsd-executor.md must explicitly forbid `git stash pop` — the load-bearing footgun (#3542)', + ); + assert.match( + content, + /`git stash apply`/, + 'gsd-executor.md must explicitly forbid `git stash apply` — same shared-stack hazard as pop (#3542)', + ); + assert.match( + content, + /`git stash drop`/, + 'gsd-executor.md must explicitly forbid `git stash drop` — mutates the shared stack (#3542)', + ); + + // The prohibition must explain WHY (shared storage across worktrees) so + // the agent understands the failure mode rather than treating it as an + // arbitrary rule. + assert.match( + content, + /shared|share[d]?\s+(across|between)/i, + 'gsd-executor.md must document that stash storage is shared across worktrees (#3542)', + ); + + // The prohibition must document at least one alternative the agent CAN + // use to inspect or move work between refs without touching `refs/stash`. + // The triage brief proposes commit-to-throwaway-branch OR read-only + // `git show :` / `git diff -- `. + const hasThrowawayBranch = /throwaway[- ]branch|temp(?:orary)?[- ]?branch|scratch[- ]branch/i.test( + content, + ); + const hasGitShow = /`git show /i.test(content); + const hasGitDiffRef = /`git diff [^`]*\$?\{?ref\}?|`git diff [A-Z]+:/i.test(content); + assert.ok( + hasThrowawayBranch || hasGitShow || hasGitDiffRef, + 'gsd-executor.md must document an alternative to `git stash` ' + + '(commit-to-throwaway-branch, or read-only `git show :` / ' + + '`git diff -- `) so the agent has a sanctioned escape path (#3542)', + ); + + // The issue number must appear so future readers can trace the rule to + // its incident. + assert.match( + content, + /#3542/, + 'gsd-executor.md must reference issue #3542 next to the stash prohibition for traceability', + ); +}); + +// ─── Test B — behavioural pin of the git invariant ───────────────────────── + +test('bug-3542: stash pushed in main checkout is visible inside a linked worktree', () => { + const tmpRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'bug-3542-stash-'))); + const mainRepo = path.join(tmpRoot, 'main'); + const linkedWorktree = path.join(tmpRoot, 'wt'); + + try { + // Set up a normal repo with one commit. + fs.mkdirSync(mainRepo); + const gitOpts = { cwd: mainRepo, stdio: 'pipe' }; + execSync('git init -q', gitOpts); + execSync('git config user.email "test@test.com"', gitOpts); + execSync('git config user.name "Test"', gitOpts); + execSync('git config commit.gpgsign false', gitOpts); + fs.writeFileSync(path.join(mainRepo, 'a.txt'), 'initial\n'); + execSync('git add a.txt', gitOpts); + execSync('git commit -q -m initial', gitOpts); + + // Create a linked worktree on a separate branch — this is what the + // executor agent runs inside. + execSync(`git worktree add -q "${linkedWorktree}" -b wt-branch`, gitOpts); + + // Push a stash from the MAIN checkout (simulating a prior session). + fs.writeFileSync(path.join(mainRepo, 'a.txt'), 'wip in main\n'); + execSync('git stash push -q -u -m "from-main-checkout"', gitOpts); + + // Sanity check: the stash exists in the main checkout's view. + const mainList = execSync('git stash list', { cwd: mainRepo }).toString(); + assert.match( + mainList, + /from-main-checkout/, + 'pre-condition: main checkout must see its own stash entry', + ); + + // The load-bearing assertion: the linked worktree sees the same + // stash entry, even though it was pushed from a different working + // tree. This is the invariant that makes `git stash pop` inside an + // executor agent's worktree an isolation violation. + const worktreeList = execSync('git stash list', { + cwd: linkedWorktree, + }).toString(); + assert.match( + worktreeList, + /from-main-checkout/, + 'bug #3542 invariant: stash entries pushed from any worktree (or the ' + + 'main checkout) are visible in every linked worktree, because ' + + '`refs/stash` lives in the shared parent .git directory. If this ' + + 'assertion ever stops holding (e.g. git introduces per-worktree ' + + 'stash storage in a future release), the executor agent prohibition ' + + 'in agents/gsd-executor.md can be relaxed.', + ); + + // Stronger pin: a `git stash pop` inside the worktree must actually + // pop the stash pushed from main — proving cross-worktree mutation, + // not just visibility. We pop into a clean working tree on a + // different branch, so any applied content is the contamination. + execSync('git stash pop -q', { cwd: linkedWorktree, stdio: 'pipe' }); + const popped = fs.readFileSync(path.join(linkedWorktree, 'a.txt'), 'utf-8'); + assert.strictEqual( + popped, + 'wip in main\n', + 'bug #3542 invariant: `git stash pop` inside a linked worktree applies ' + + 'a stash pushed in the main checkout — proving the shared-stack ' + + 'contamination the executor prohibition exists to prevent.', + ); + } finally { + fs.rmSync(tmpRoot, { recursive: true, force: true }); + } +}); From 0d9a800e564b9228564e38b98a4fdb9684358ed5 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 15 May 2026 08:31:19 -0400 Subject: [PATCH 2/2] chore(3542): backfill PR number in changeset Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/3542-executor-git-stash-prohibition.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/3542-executor-git-stash-prohibition.md b/.changeset/3542-executor-git-stash-prohibition.md index 6439f5065..05ea2826f 100644 --- a/.changeset/3542-executor-git-stash-prohibition.md +++ b/.changeset/3542-executor-git-stash-prohibition.md @@ -1,5 +1,5 @@ --- type: Fixed -pr: 0 +pr: 3546 --- **Executor agents are now forbidden from running any `git stash` subcommand inside worktree isolation** — git stores stashes at `refs/stash` in the parent `.git/` directory, so the stash list is shared across the main checkout and every linked worktree. A `git stash pop` inside an executor's worktree silently applied WIP pushed from a prior sibling-worktree session, producing UU/UD merge-conflict states and phantom untracked files that violated the `isolation="worktree"` invariant. The `` block in `agents/gsd-executor.md` now lists the full `git stash` family alongside `--no-verify`, `--hard`, and `git update-ref`, with sanctioned alternatives (commit to a throwaway branch you own, or read-only `git show :` / `git diff -- `). The orchestrator-side post-wave-hook helper in `execute-phase.md` continues to use stash deliberately and is now annotated to make the single-checkout precondition explicit. Closes #3542.