Skip to content

docs(implementation): document TDD project-policy override#1054

Open
kyle-sexton wants to merge 2 commits into
mainfrom
codex/406-document-tdd-off-switch
Open

docs(implementation): document TDD project-policy override#1054
kyle-sexton wants to merge 2 commits into
mainfrom
codex/406-document-tdd-off-switch

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #406

Summary

Documents the implementation plugin's existing project-level testing-cadence override without changing its advertised TDD-by-default identity. TDD remains the fallback when a consuming project is silent; an explicit testing policy in the project's CLAUDE.md or rules now clearly takes precedence in every execution mode.

This gives teams a discoverable, repository-scoped opt-out while keeping the plugin's zero-configuration default unchanged.

Fix

  • Add a concrete tests-after override example to the implementation README's Configuration section.
  • State in the central /implementation:implement cadence that project policy overrides both the fallback and mode-context guidance.
  • Make the feature and bugfix contexts explicitly honor project testing policy before their test-first fallback instructions.
  • Remove the contradictory bugfix phrase test-first, always.
  • Bump the implementation plugin from 0.7.8 to 0.7.9 and add its changelog entry.
  • Keep a user-global userConfig key deferred, matching the accepted operator decision.

Research

The implementation follows Anthropic's current official scope model:

  • How Claude remembers your project identifies project CLAUDE.md as team-shared instruction space for build/test instructions, coding standards, and common workflows.
  • Plugins reference documents userConfig as user/managed-scoped and states that project and local plugin-option entries are ignored.

Together, those sources support keeping testing cadence in project instructions rather than introducing a user-scoped plugin option.

Verification

  • Independent verification caught two contradictory mode-context instructions before publication; both were corrected and the resulting six-file change was re-verified before publish.
  • Focused contract assertion: README, central skill, feature context, and bugfix context agree that project policy takes precedence while TDD remains the silent-project fallback.
  • Regression assertion: bugfix context no longer contains test-first, always.
  • claude plugin validate plugins/implementation — passed before and after rebasing.
  • git diff --check origin/main...HEAD — passed after rebasing.
  • Direct changelog assertion — verified 0.7.8 → 0.7.9 and a newly added ## [0.7.9] entry.
  • Direct changed-skill assertion — frontmatter/routing description preserved; skill remains below the 500-line cap.
  • Live collision audit — no open PR files or other registered worktrees touch plugins/implementation/**.
  • Rebased cleanly onto current origin/main at b123108ed745c51478d77737a2a01480623fd2ae.

Local disclosure: the repository's canonical Bash changed-skill/changelog wrappers and markdownlint were not completed locally. The default bash command resolves to an unconfigured WSL environment, direct Git Bash wrapper attempts exceeded the strict local timebox, and no standalone markdownlint binary is installed. The focused direct equivalents above passed; canonical wrappers and markdownlint are left to required CI.

Related

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 32s —— View job


Code Review

  • Gather context and understand request
  • Read changed files
  • Verify consistency across all changed files
  • Check alignment with CLAUDE.md guidelines
  • Post review findings

This is a well-scoped, well-executed documentation PR. The core objective — making the existing project-policy override explicitly discoverable across all execution surfaces without changing the TDD-by-default identity — is accomplished cleanly. Here are my findings, most significant first.


Minor issue: bugfix.md "Common pitfalls" contradicts the updated step 1

The step 1 heading was correctly updated to qualify test-first with "when project policy is silent," but the Common pitfalls section retains an unqualified imperative:

- **Fixing without a test** — "I can see the bug, the fix is obvious" leads to regressions. Write the test first

plugins/implementation/skills/implement/context/bugfix.md line 20

This creates a quiet contradiction: a consumer with Use tests-after in their CLAUDE.md would see step 1 correctly defer to their policy, but then encounter a pitfall entry telling them "Write the test first" with no qualification. Suggesting a small addition like "Write the test first (unless project policy specifies otherwise)" or "Under the test-first fallback, write the test first" to keep the pitfall section honest. Fix this →


Observation: SKILL.md frontmatter and plugin.json description still say "TDD by default"

The skill frontmatter description and plugin.json description both still read "TDD by default" / "TDD-by-default cadence" without the "when project policy is silent" qualifier:

These are one-line summary fields where brevity is appropriate, so "TDD by default" reads as acceptable shorthand. The PR description explicitly notes it wants to preserve the plugin's "TDD-by-default identity," so this is likely intentional. Not blocking — just confirming the trade-off was deliberate.


Positive observations

Consistency across the four updated surfaces. README, SKILL.md (step 3), feature.md (step 5), and bugfix.md (step 1) all use the same priority model — project policy first, TDD as the fallback — and the SKILL.md step 3 even embeds the exact opt-out sentence shown in the README (Use tests-after for implementation work; do not use test-first TDD.), which is a nice touch that makes the documentation self-reinforcing.

refactor.md left untouched correctly. It contains no test-first instructions, so there was nothing to qualify there.

Semver is appropriate. A 0.7.8 → 0.7.9 patch bump is the right call for a documentation/clarification-only change with no behavior change and no breaking API surface.

CHANGELOG is accurate and well-structured. The ## [0.7.9] entry under ### Changed correctly scopes the change and cross-references the closing issue.

The phrase "test-first, always" is gone. The old bugfix.md heading absolute was the sharpest contradiction; its removal is correct.


Summary

One minor fix is worth making before merge: the bugfix.md Common pitfalls entry needs its own project-policy qualifier to match the updated step 1. Everything else is clean.

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 30s —— View job


Security Review — PR #1054

  • Read the PR diff
  • Review changed files for security issues
  • Post findings

Findings

No security issues found.

All six changed files are pure documentation and metadata:

File Change
plugin.json Semver bump 0.7.80.7.9 — no schema or permission changes
CHANGELOG.md Prose changelog entry — no executable content
README.md Adds a Markdown code block example for a project instruction — static text only
SKILL.md Rewrites step 3 prose — no new tool invocations, permissions, or external references
context/bugfix.md Rewrites step 1 prose — same scope as above
context/feature.md Rewrites step 5 prose — same scope as above

None of the changes introduce code execution paths, new permissions, external URLs, credential handling, injection surfaces, supply-chain dependencies, or GitHub Actions modifications. The diff presents no attack surface beyond the existing plugin runtime that loads these files.

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 17s —— View job


Code Review — PR #1054 (re-review after 9925c11)

  • Gather context
  • Read changed files
  • Verify the new commit addresses previous findings
  • Check consistency across all surfaces
  • Post review findings

The second commit (9925c11 docs(implementation): qualify bugfix testing pitfall) directly and cleanly resolves the one minor issue flagged in the prior review. Here is the full picture.


Previous finding resolved ✓

bugfix.md Common pitfalls — now correctly qualified.

The unqualified "Write the test first" in the pitfall entry has been replaced with:

Under the test-first fallback, write the test first; otherwise follow the project's declared CLAUDE.md / rules testing cadence

plugins/implementation/skills/implement/context/bugfix.md line 20

The fix is precise — it scopes the imperative to the fallback cadence and points to the same authority (project CLAUDE.md / rules) used throughout the other updated surfaces. The vocabulary is consistent with step 1.


Consistency check across all four updated surfaces

All surfaces now agree: project policy wins; TDD is the fallback when the project is silent.

Surface Verdict
README Configuration section ✅ "TDD remains the fallback when the consuming project's CLAUDE.md or rules do not declare another cadence."
SKILL.md step 2 item 3 ✅ "TDD is the fallback when the consuming project does not declare another testing cadence … A consumer can opt out in its CLAUDE.md / rules"
feature.md step 5 ✅ "first honor the consuming project's testing cadence … that project policy overrides the test-first instructions in this step"
bugfix.md step 1 + pitfalls ✅ Both now qualified; "test-first, always" is gone

Unchanged items confirmed intentional

  • SKILL.md frontmatter: still "TDD by default" in the one-line description. Brevity is appropriate for a summary field; the full qualifier lives in step 2. The PR description explicitly states this preserves the plugin's "TDD-by-default identity" — the trade-off is deliberate.
  • plugin.json description: still "TDD-by-default cadence" for the same reason.
  • refactor.md: left untouched. It carries no test-first instructions, so there is nothing to qualify.

Semver and changelog


Verdict

Both commits are clean. No outstanding issues. This PR is ready to merge.

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 51s —— View job


Security Review — PR #1054

  • Read PR diff
  • Review changed files for security issues
  • Post findings

Findings

No security issues found.

All six changed files are pure documentation and metadata:

File Change
plugin.json Semver bump 0.7.80.7.9 — no schema, permission, or keyword changes
CHANGELOG.md Prose changelog entry — no executable content
README.md Adds a Markdown code-block example (Use tests-after for implementation work…) — static text only; rendered as a quoted code block, not injected into any execution context
SKILL.md Rewrites step 3 prose to add project-policy precedence language — no new shell commands, tool invocations, external URLs, or permission grants
context/bugfix.md Rewrites step 1 and common pitfalls with the same project-policy qualifier — same scope as above
context/feature.md Rewrites step 5 prose — same scope as above

Pre-computed shell context (!git branch --show-current, !git status --porcelain, etc.) in SKILL.md is unchanged from the pre-PR version — no new shell execution surfaces were introduced.

Prompt-injection surface: the skill .md files are instructions loaded into Claude's context at invocation. The new prose directs the model to honor consumer CLAUDE.md instructions — this is the documented, intentional trust model for Claude Code plugins (project CLAUDE.md overrides plugin defaults). No adversarially-constructed instruction is embedded; the example opt-out string is enclosed in a fenced code block in the README and in a plain-English sentence in the skill files, neither of which creates a novel injection path beyond what already exists in the plugin system design.

None of the changes introduce code execution paths, new permissions, external URLs, credential handling, injection surfaces, supply-chain dependencies, or GitHub Actions modifications.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9925c11724

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

## Sequence

1. **Reproduce first (test-first, always)** — write a failing test that demonstrates the bug before touching any production code. If you can't reproduce it in a test, you can't prove you fixed it. Test name should describe the bug: `Should_ReturnError_When_InputIsNull`, not `TestFix42`. Invoke `/tdd:principles` via Skill tool (when the `tdd` plugin is installed) for test design guidance (what kind of test, where it goes, what to assert). Bug fixes are the strongest case for test-first — the failing test IS the bug report
1. **Reproduce first (test-first by default when project policy is silent)** — first honor the consuming project's testing cadence from its `CLAUDE.md` / rules; that project policy overrides the test-first instructions in this step. When the project declares no cadence, write a failing test that demonstrates the bug before touching any production code. If you can't reproduce it in a test, you can't prove you fixed it. Test name should describe the bug: `Should_ReturnError_When_InputIsNull`, not `TestFix42`. Invoke `/tdd:principles` via Skill tool (when the `tdd` plugin is installed) for test design guidance (what kind of test, where it goes, what to assert). Under the fallback cadence, bug fixes are the strongest case for test-first — the failing test IS the bug report

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Align bugfix opt-out with later steps

When a repo declares a tests-after cadence, this new branch skips creating the step-1 failing test, but the rest of the bugfix sequence still depends on that artifact (the failing test from step 1 should now pass and the failing-test checkpoint). That leaves the documented opt-out internally inconsistent for bug fixes and can push agents back into test-first despite the project policy; make the later verification/checkpoints conditional or spell out the tests-after bugfix sequence.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Finding Classification Evidence Reacted
1 Bugfix opt-out is internally inconsistent: step 1 now defers test-first to project policy, but step 4 ("the failing test from step 1 should now pass") and the first Checkpoint still assume that step-1 failing test unconditionally VALID At head 9925c11, step 1 defers to the consuming project's CLAUDE.md/rules cadence, but context/bugfix.md step 4 and the "Failing test committed" checkpoint reference the step-1 failing test with no otherwise-style hedge — the same gap step 1 and the Common pitfalls entry already close. Under a project-declared tests-after cadence the step-1 test is never written, so those two spots are internally inconsistent. 👍

Scope (safe-tier babysit): defer-only, not a prescribed tests-after sequence. Applying the same hedge already used in step 1 and the Common pitfalls entry; deliberately not spelling out a tests-after bugfix flow, since that would resolve a design tension this PR leaves open (step 1 itself calls bug fixes "the strongest case for test-first — the failing test IS the bug report"). Suggested edits:

  • Step 4**Verify the fix** — under the test-first fallback, the failing test from step 1 should now pass; otherwise verify per the project's declared CLAUDE.md / rules testing cadence. Run the full test suite for the affected project — your fix may have side effects
  • First checkpointFailing test committed first (proves the bug exists — optional but valuable for git history; applies under the test-first fallback, not a project-declared tests-after cadence)

I could not land the commit this cycle: a PostToolUse formatter hook on this machine deletes tool-written files inside the babysit worktree (git-written files persist), so the edit above is left for the author or a later clean run to apply. Not resolving this thread (safe tier).

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

🔒 babysit-prs lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, safe tier). Will fix clear branch-owned findings and push; will not resolve threads or merge (safe tier).

@kyle-sexton kyle-sexton added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implementation: TDD-by-default fires when consumer CLAUDE.md is silent — no userConfig toggle

1 participant