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 1 commit 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>
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".
| case "$COMMAND" in | ||
| git\ commit*) ;; # Continue to check | ||
| *) exit 0 ;; # Not a commit, allow immediately |
There was a problem hiding this comment.
Detect commit invocations beyond literal
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.
Bind verification marker to staged content, not paths
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
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"
}
]
}
}|
|
||
| # 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.
|
|
||
| - 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 |
|
|
||
| ## 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.** |
|
|
||
| - 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 |
| 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 ;;
esac| # 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)| ``` | ||
|
|
||
| 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.
| - 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.
|
|
||
| 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.
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