fix: reconcile planner action contract (#3326)

* fix: reconcile planner action contract

The deep_work_rules workflow block was stronger than the planner agent contract: it required verbatim context copies and self-sufficient action text, so planners were incentivized to inline implementation code. Bound action content to directive prose with concrete identifiers, allow behavior/test acceptance criteria, and pin the cross-file contract with a regression test. Closes #3320.

* chore: add changeset for planner contract fix

.changeset/eager-elks-purr.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3326
+---
+Reconciled /gsd-plan-phase deep_work_rules with the gsd-planner action contract so planners keep action blocks as directive prose, avoid fenced implementation dumps, and allow behavior/test acceptance criteria alongside source assertions. (#3320)

agents/gsd-planner.md

@@ -198,8 +198,10 @@ Every task has four required fields:
 - Bad: "the auth files", "relevant components"
 
 **<action>:** Specific implementation instructions, including what to avoid and WHY.
-- Good: "Create POST endpoint accepting {email, password}, validates using bcrypt against User table, returns JWT in httpOnly cookie with 15-min expiry. Use jose library (not jsonwebtoken - CommonJS issues with Edge runtime)."
+- Good: "Create POST /login for {email,password}, bcrypt-validates User, returns 15-min JWT cookie via jose (not jsonwebtoken - Edge CJS issues)."
 - Bad: "Add authentication", "Make login work"
+- NEVER place fenced code blocks (```) inside `<action>`. Action is directive prose, not implementation code.
+- Code excerpts belong in `<read_first>` source files or referenced context. Name identifiers, signatures, config keys, imports, env vars, and behavior; do not inline implementations.
 
 **<verify>:** How to prove the task is complete.
 
@@ -213,9 +215,9 @@ Every task has four required fields:
 - Bad: "It works", "Looks good", manual-only verification
 - Simple format also accepted: `npm test` passes, `curl -X POST /api/auth/login` returns 200
 
-**Nyquist Rule:** Every `<verify>` must include an `<automated>` command. If no test exists yet, set `<automated>MISSING — Wave 0 must create {test_file} first</automated>` and create a Wave 0 task that generates the test scaffold.
+**Nyquist Rule:** Every `<verify>` includes `<automated>`. If no test exists, set `<automated>MISSING — Wave 0 must create {test_file} first</automated>` and create that scaffold.
 
-**Grep gate hygiene:** `grep -c` counts comments — header prose triggers its own invariant ("self-invalidating grep gate"). Use `grep -v '^#' | grep -c token`. Bare `== 0` gates on unfiltered files are forbidden.
+**Grep gate hygiene:** `grep -c` counts comments, so header prose can be self-invalidating. Use `grep -v '^#' | grep -c token`. Bare `== 0` gates on unfiltered files are forbidden.
 
 **<done>:** Acceptance criteria - measurable state of completion.
 - Good: "Valid credentials return 200 + JWT cookie, invalid credentials return 401"

get-shit-done/workflows/plan-phase.md

@@ -878,22 +878,24 @@ Every task MUST include these fields — they are NOT optional:
    - Any file whose patterns, signatures, types, or conventions must be replicated or respected
 
 2. **`<acceptance_criteria>`** — Verifiable conditions that prove the task was done correctly. Rules:
-   - Every criterion must be checkable with grep, file read, test command, or CLI output
+   - Every criterion must be checkable as a source assertion, behavior assertion, test command, or CLI output
    - NEVER use subjective language ("looks correct", "properly configured", "consistent with")
-   - ALWAYS include exact strings, patterns, values, or command outputs that must be present
+   - Include exact strings, patterns, values, command outputs, or observable behavior where that is the right proof
    - Examples:
      - Code: `auth.py contains def verify_token(` / `test_auth.py exits 0`
+     - Behavior: `POST /api/auth/login returns 200 + httpOnly JWT cookie for valid credentials`
      - Config: `.env.example contains DATABASE_URL=` / `Dockerfile contains HEALTHCHECK`
      - Docs: `README.md contains '## Installation'` / `API.md lists all endpoints`
      - Infra: `deploy.yml has rollback step` / `docker-compose.yml has healthcheck for db`
 
 3. **`<action>`** — Must include CONCRETE values, not references. Rules:
    - NEVER say "align X with Y", "match X to Y", "update to be consistent" without specifying the exact target state
-   - ALWAYS include the actual values: config keys, function signatures, SQL statements, class names, import paths, env vars, etc.
-   - If CONTEXT.md has a comparison table or expected values, copy them into the action verbatim
-   - The executor should be able to complete the task from the action text alone, without needing to read CONTEXT.md or reference files (read_first is for verification, not discovery)
+   - Include concrete identifiers and reference values: config keys, function signatures, SQL table names, class names, import paths, env vars, endpoint paths, etc.
+   - If CONTEXT.md has a comparison table or expected values, copy only the target identifiers/values needed to remove ambiguity
+   - Do not include full file contents, fenced code blocks, or complete implementations in `<action>`
+   - The executor should understand the intended target state from `<action>` and use `<read_first>` files for current implementation details, patterns, and source-of-truth context
 
-**Why this matters:** Executor agents work from the plan text. Vague instructions like "update the config to match production" produce shallow one-line changes. Concrete instructions like "add DATABASE_URL=postgresql://... , set POOL_SIZE=20, add REDIS_URL=redis://..." produce complete work. The cost of verbose plans is far less than the cost of re-doing shallow execution.
+**Why this matters:** Executor agents work from the plan text. Vague instructions like "update the config to match production" produce shallow one-line changes. Concrete instructions like "add DATABASE_URL, set POOL_SIZE=20, add REDIS_URL, and read config/runtime.ts before editing" produce complete work without turning the planner into the executor.
 </deep_work_rules>
 
 <quality_gate>
@@ -901,8 +903,8 @@ Every task MUST include these fields — they are NOT optional:
 - [ ] Each plan has valid frontmatter
 - [ ] Tasks are specific and actionable
 - [ ] Every task has `<read_first>` with at least the file being modified
-- [ ] Every task has `<acceptance_criteria>` with grep-verifiable conditions
-- [ ] Every `<action>` contains concrete values (no "align X with Y" without specifying what)
+- [ ] Every task has `<acceptance_criteria>` with behavior, test-command, CLI, or source assertions
+- [ ] Every `<action>` contains concrete identifiers without fenced code blocks or full implementations
 - [ ] Dependencies correctly identified
 - [ ] Waves assigned for parallel execution
 - [ ] must_haves derived from phase goal

tests/bug-3320-planner-deep-work-rules.test.cjs

@@ -0,0 +1,92 @@
+'use strict';
+
+// allow-test-rule: source-text-is-product [#3320]
+// The bug is a contradiction in prompt/workflow source text. These assertions
+// intentionally pin the contract words that planner agents consume.
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+
+const ROOT = path.join(__dirname, '..');
+const PLANNER_AGENT = path.join(ROOT, 'agents', 'gsd-planner.md');
+const PLAN_PHASE_WORKFLOW = path.join(ROOT, 'get-shit-done', 'workflows', 'plan-phase.md');
+
+function read(relativePath) {
+  return fs.readFileSync(path.join(ROOT, relativePath), 'utf8');
+}
+
+function extractDeepWorkRules() {
+  const workflow = fs.readFileSync(PLAN_PHASE_WORKFLOW, 'utf8');
+  const match = workflow.match(/<deep_work_rules>[\s\S]*?<\/deep_work_rules>/);
+  assert.ok(match, 'plan-phase.md must contain a deep_work_rules block');
+  return match[0];
+}
+
+describe('bug #3320 planner action contract', () => {
+  test('planner agent explicitly keeps implementation code out of action blocks', () => {
+    const planner = fs.readFileSync(PLANNER_AGENT, 'utf8');
+
+    assert.match(
+      planner,
+      /NEVER place fenced code blocks \(```\) inside `<action>`/,
+      'gsd-planner.md must explicitly forbid fenced implementation code in <action>'
+    );
+    assert.match(
+      planner,
+      /Code excerpts belong in `<read_first>` source files or referenced context/,
+      'gsd-planner.md must route code excerpts to context/read-first material'
+    );
+  });
+
+  test('plan-phase deep_work_rules no longer requires self-sufficient code dumps', () => {
+    const deepWorkRules = extractDeepWorkRules();
+
+    assert.doesNotMatch(
+      deepWorkRules,
+      /copy them into the action verbatim/,
+      'deep_work_rules must not tell planners to copy source material verbatim into <action>'
+    );
+    assert.doesNotMatch(
+      deepWorkRules,
+      /complete the task from the action text alone/,
+      'deep_work_rules must not make <action> self-sufficient without read_first/context'
+    );
+    assert.match(
+      deepWorkRules,
+      /Do not include full file contents, fenced code blocks, or complete implementations in `<action>`/,
+      'deep_work_rules must explicitly bound concrete values to avoid code dumping'
+    );
+  });
+
+  test('plan-phase acceptance criteria allow behavior and test assertions', () => {
+    const deepWorkRules = extractDeepWorkRules();
+
+    assert.match(
+      deepWorkRules,
+      /behavior assertion/,
+      'acceptance criteria must allow behavior assertions, not just grep checks'
+    );
+    assert.match(
+      deepWorkRules,
+      /test command/,
+      'acceptance criteria must allow test-command assertions'
+    );
+  });
+
+  test('quality gate matches the reconciled planner contract', () => {
+    const workflow = read('get-shit-done/workflows/plan-phase.md');
+
+    assert.match(
+      workflow,
+      /Every task has `<acceptance_criteria>` with behavior, test-command, CLI, or source assertions/,
+      'quality gate must not narrow acceptance criteria back to grep-only checks'
+    );
+    assert.match(
+      workflow,
+      /Every `<action>` contains concrete identifiers without fenced code blocks or full implementations/,
+      'quality gate must enforce concrete prose without implementation dumps'
+    );
+  });
+});