chore: add spec-drift detection commands and pre-commit hook#649
chore: add spec-drift detection commands and pre-commit hook#649ciscoRankush wants to merge 2 commits intonextfrom
Conversation
Add automated SDD documentation drift detection tooling: - /spec-drift: full scan command for validating all ai-docs against source code - /spec-drift-changed: incremental scan for changed files only - Pre-commit hook to block commits when ai-docs drift is unverified Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17636199d6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
.claude/hooks/check-ai-docs-drift.sh
Outdated
| case "$COMMAND" in | ||
| git\ commit*) ;; # Continue to check | ||
| *) exit 0 ;; # Not a commit, allow immediately |
There was a problem hiding this comment.
git commit
The hook only enforces validation when the command string starts with git commit, so commit flows like git -c user.name=... commit ..., HUSKY=0 git commit ..., or chained commands such as git add . && git commit ... will bypass this guard and allow unverified contact-center changes to be committed. Since this script is meant to be the blocking gate, matching only one command shape leaves a real enforcement hole.
Useful? React with 👍 / 👎.
| esac | ||
|
|
||
| # Get staged files under the contact-center package | ||
| STAGED_CC=$(git diff --cached --name-only 2>/dev/null | grep "^${CC_PKG}/") |
There was a problem hiding this comment.
The marker hash is derived from git diff --cached --name-only output, which means it changes only when the set of staged file paths changes; if a developer runs /spec-drift-changed, then edits and restages the same files, the existing marker still matches and the commit is allowed without re-validating docs against the new content. This weakens the drift check and can let stale verification pass.
Useful? React with 👍 / 👎.
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Shreyas281299
left a comment
Shreyas281299
left a comment
There was a problem hiding this comment.
Review Summary
| Severity | Count |
|---|---|
| High | 3 |
| Medium | 3 |
| Low | 6 |
Recommendation: Request Changes -- The 3 High severity issues should be addressed before merge:
- Register the hook in
settings.json(otherwise the hook is dead code) - Fix "Task tool" -> "Agent tool" references (otherwise the commands will fail)
- Add
python3availability check (otherwise the hook silently becomes a no-op)
Positive Observations
- Well-structured command files with clear step-by-step instructions and output format templates
- The 7-category drift detection framework is comprehensive and well-defined
- Good use of severity levels (Blocking/Important/Medium/Minor) in the drift report
- The incremental scan (
spec-drift-changed) is a practical addition for pre-commit workflows - The shell hook logic is concise and handles the common cases correctly
| @@ -0,0 +1,49 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
HIGH: Hook is not registered in settings.json -- dead code
How will we set this? The hook script exists but the PR does NOT include any changes to .claude/settings.json or .claude/settings.local.json to register it as a PreToolUse hook. The current .claude/settings.local.json only contains permissions -- no hooks configuration. Without registration, this hook will never execute and the entire gating mechanism is non-functional.
Suggested fix: Add hook registration to .claude/settings.json:
{
"hooks": {
"PreToolUse": [
{
"matcher": "Bash",
"command": ".claude/hooks/check-ai-docs-drift.sh"
}
]
}
}There was a problem hiding this comment.
Fixed in 3cf0a3f. Added .claude/settings.json with the PreToolUse hook registration for Bash commands.
|
|
||
| # Read tool input from stdin (JSON with tool_input.command) | ||
| INPUT=$(cat) | ||
| COMMAND=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('tool_input',{}).get('command',''))" 2>/dev/null) |
There was a problem hiding this comment.
HIGH: Silent failure when python3 is unavailable
The script uses python3 -c to parse JSON from stdin, with 2>/dev/null suppressing errors. If python3 is not installed, COMMAND silently becomes an empty string, the case statement falls through to *) exit 0, and the hook always passes -- silently disabling the gate with no warning.
Suggested fix: Add a prerequisite check before this line:
command -v python3 >/dev/null 2>&1 || { echo "python3 required for spec-drift hook" >&2; exit 0; }Alternatively, use jq or pure bash string matching for the simpler JSON structure.
There was a problem hiding this comment.
Fixed in 3cf0a3f. Added python3 availability check (command -v python3) and a fail-closed guard — if COMMAND is empty after parsing, the hook now exits with code 2 (block) instead of 0 (allow).
|
|
||
| - Do NOT auto-fix anything — report findings only | ||
| - Always read actual source code to verify — never assume | ||
| - Use the Task tool with `subagent_type: "Explore"` for checker agents |
There was a problem hiding this comment.
HIGH: References non-existent "Task tool" instead of "Agent tool"
While a TaskCreate tool exists in Claude Code, it is for task list/progress tracking and does not accept a subagent_type parameter. The subagent_type: "Explore" parameter belongs to the Agent tool. Claude would fail or do the wrong thing when trying to follow this instruction.
Suggested fix:
| - Use the Task tool with `subagent_type: "Explore"` for checker agents | |
| - Use the Agent tool with `subagent_type: "Explore"` for checker agents |
There was a problem hiding this comment.
Fixed in 3cf0a3f. Changed all references from "Task tool" to "Agent tool" with subagent_type: "Explore".
.claude/commands/spec-drift.md
Outdated
|
|
||
| ## Step 2: Spawn Checker Agents in Parallel | ||
|
|
||
| Use the Task tool to spawn agents. **All agents run in parallel.** |
There was a problem hiding this comment.
HIGH: References non-existent "Task tool" instead of "Agent tool"
Same issue as in spec-drift-changed.md. TaskCreate is for task tracking, not for spawning agents. The subagent_type: "Explore" parameter belongs to the Agent tool.
Suggested fix:
| Use the Task tool to spawn agents. **All agents run in parallel.** | |
| Use the Agent tool to spawn agents. **All agents run in parallel.** |
There was a problem hiding this comment.
Fixed in 3cf0a3f. Changed to "Agent tool" here as well.
.claude/commands/spec-drift.md
Outdated
|
|
||
| - Do NOT auto-fix anything — report findings only | ||
| - Always read actual source code to verify — never assume | ||
| - Use the Task tool with `subagent_type: "Explore"` for all checker agents |
There was a problem hiding this comment.
HIGH: Same "Task tool" → "Agent tool" fix needed here
Suggested fix:
| - Use the Task tool with `subagent_type: "Explore"` for all checker agents | |
| - Use the Agent tool with `subagent_type: "Explore"` for all checker agents |
There was a problem hiding this comment.
Fixed in 3cf0a3f. Updated to "Agent tool" in the Rules section as well.
| COMMAND=$(echo "$INPUT" | python3 -c "import sys,json; print(json.load(sys.stdin).get('tool_input',{}).get('command',''))" 2>/dev/null) | ||
|
|
||
| # Only gate git commit commands | ||
| case "$COMMAND" in |
There was a problem hiding this comment.
LOW: Overly broad case pattern for commit detection
The pattern git\ commit* matches any command starting with "git commit", including git plumbing commands like git commit-tree. While unlikely to cause issues in practice, a more precise pattern would be:
case "$COMMAND" in
"git commit"|git\ commit\ *) ;; # Match 'git commit' standalone or with args
*) exit 0 ;;
esacThere was a problem hiding this comment.
Fixed in 3cf0a3f. Updated case pattern to "git commit"|git\ commit\ * to avoid matching git commit-tree and similar plumbing commands.
| # Compute hash of all staged contact-center files | ||
| HASH=$(echo "$STAGED_CC" | sort | shasum | cut -d' ' -f1) | ||
| MARKER="/tmp/.spec-drift-verified-${HASH}" | ||
|
|
There was a problem hiding this comment.
LOW: shasum portability across platforms
shasum (SHA-1 default) is macOS-native. On some Linux distributions, only sha256sum is available. For cross-platform teams, consider a fallback:
HASH=$(echo "$STAGED_CC" | sort | (shasum 2>/dev/null || sha256sum) | cut -d' ' -f1)There was a problem hiding this comment.
Fixed in 3cf0a3f. Added (shasum 2>/dev/null || sha256sum) fallback in both the hook and the marker creation in spec-drift-changed.md.
| ``` | ||
|
|
||
| For task widget source files, map to the widget-specific ai-docs: | ||
| - `packages/contact-center/task/src/widgets/CallControl/*` → `packages/contact-center/task/ai-docs/widgets/CallControl/` |
There was a problem hiding this comment.
LOW: Hardcoded widget path mappings
The mapping for task widget source files to ai-docs is hardcoded to 4 specific widgets (CallControl, IncomingTask, OutdialCall, TaskList). New widgets added to task/src/widgets/ would not be automatically detected.
Suggested fix: Add a dynamic pattern instruction:
For any widget under packages/contact-center/task/src/widgets/{WidgetName}/*,
map to packages/contact-center/task/ai-docs/widgets/{WidgetName}/ if the
ai-docs folder exists.
There was a problem hiding this comment.
Fixed in 3cf0a3f. Replaced the 4 hardcoded widget paths with a dynamic pattern: packages/contact-center/task/src/widgets/{WidgetName}/* → packages/contact-center/task/ai-docs/widgets/{WidgetName}/ (if ai-docs folder exists). New widgets are automatically covered.
| - Always read actual source code to verify — never assume | ||
| - Use the Task tool with `subagent_type: "Explore"` for checker agents | ||
| - Run agents in parallel when multiple folders are affected | ||
| - Always create the verification marker at the end, even if there are findings (the developer decides whether to fix or commit as-is) |
There was a problem hiding this comment.
LOW: Marker created regardless of blocking findings (design observation)
This rule makes the tool purely advisory -- even "Blocking" severity findings (described as "wrong API that would cause runtime errors") won't actually block the commit. Consider documenting this advisory-only nature more prominently in the report output, so developers understand the tool's stance.
There was a problem hiding this comment.
Fixed in 3cf0a3f. Updated the rule to explicitly state: "this tool is advisory: the developer decides whether to fix or commit as-is".
.claude/commands/spec-drift.md
Outdated
|
|
||
| For each ai-docs folder found, identify its corresponding source code directory (the parent directory of `ai-docs/`). | ||
|
|
||
| Build an inventory like: |
There was a problem hiding this comment.
LOW: Hardcoded ai-docs inventory is exemplary but could be clearer
The inventory table lists 12+ specific paths and ends with "... (discover all that exist on the current branch)". While clearly intended as illustrative, consider adding an explicit label like "Example inventory (actual results may vary):" to prevent any ambiguity.
There was a problem hiding this comment.
Fixed in 3cf0a3f. Changed to "Build an inventory (example — actual results will vary based on current branch):".
- Register hook in settings.json (was dead code without it) - Fail-closed when python3 unavailable or JSON parse fails - Use content-based hash (git diff --cached) instead of path-only hash - Don't consume marker in PreToolUse — invalidated naturally by content changes - Precise case pattern to avoid matching git commit-tree - Add shasum/sha256sum portability fallback - Fix "Task tool" → "Agent tool" references in command files - Add agent timeout handling rule - Add staged-only content note for verification marker - Make widget path mappings dynamic instead of hardcoded - Label inventory as example - Document advisory-only nature of verification Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cf0a3f6e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 1. **Repo detection**: This is ccWidgets — `packages/contact-center/` exists (no `@webex` scope) | ||
| 2. **Root AGENTS.md**: `AGENTS.md` (repo root) | ||
| 3. **Framework docs**: `ai-docs/` (README, RULES, patterns/*, templates/*) | ||
| 4. **Package-level ai-docs**: Glob for `packages/contact-center/**/ai-docs/` to find all ai-docs folders |
There was a problem hiding this comment.
packages/contact-center/**/ai-docs/ only finds directories literally named ai-docs, so in this repo it returns packages/contact-center/task/ai-docs but not packages/contact-center/task/ai-docs/widgets/{CallControl,IncomingTask,OutdialCall,TaskList}. I checked that packages/contact-center/task/ai-docs itself has no markdown files, so /spec-drift would skip every task-widget AGENTS.md/ARCHITECTURE.md and silently miss drift in that scope.
Useful? React with 👍 / 👎.
| From the changed files, build TWO lists: | ||
|
|
||
| ### List A: Changed ai-docs files | ||
| Filter for files matching `ai-docs/*.md`. These docs changed directly and need validation. |
There was a problem hiding this comment.
The literal ai-docs/*.md filter only matches markdown files directly under an ai-docs directory. In this repo the task widget docs are nested at packages/contact-center/task/ai-docs/widgets/*/*.md, so editing something like .../TaskList/AGENTS.md would not enter List A, and because List B only examines source files, /spec-drift-changed can incorrectly report “nothing to check” when the changed files are the docs themselves.
Useful? React with 👍 / 👎.
| 2. **Root AGENTS.md**: `AGENTS.md` (repo root) | ||
| 3. **Framework docs**: `ai-docs/` (README, RULES, patterns/*, templates/*) | ||
| 4. **Package-level ai-docs**: Glob for `packages/contact-center/**/ai-docs/` to find all ai-docs folders | ||
| 5. **Samples ai-docs**: Check `widgets-samples/**/ai-docs/` as well |
There was a problem hiding this comment.
The comprehensive scan only discovers package ai-docs plus widgets-samples/**/ai-docs/, but this repo also contains playwright/ai-docs/AGENTS.md and playwright/ai-docs/ARCHITECTURE.md. As written, /spec-drift never spawns a checker for the Playwright framework docs, so drift in the E2E testing guidance goes undetected even though the root AGENTS.md routes Playwright work to that scope.
Useful? React with 👍 / 👎.
2 participants
This pull request addresses
Adding automated SDD (Spec-Driven Development) documentation drift detection tooling to ensure ai-docs stay in sync with source code.
by making the following changes
.claude/commands/spec-drift.md— Full scan command that auto-discovers all ai-docs folders, spawns parallel checker agents, and validates 7 drift categories (file tree, method/API, types, events, architecture, links, code examples).claude/commands/spec-drift-changed.md— Incremental scan that only validates ai-docs affected by staged/unstaged file changes, with verification marker creation.claude/hooks/check-ai-docs-drift.sh— Pre-commit blocking hook that prevents committing when ai-docs drift is unverifiedChange Type
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that