feat: Enrich sc-test skill with Ocelot coverage workflow and harden Sondera scripts - #72
Conversation
…ondera scripts - Enriched sc-test skill with Ocelot's interactive test-coverage workflow: - Added "Ask Early, Ask Often" interactive decision points - Added Phase 2 (pattern analysis) and Phase 4 (validation) workflows - Added iterative coverage loop with user confirmation - Added detailed test writing templates for Python, TypeScript, and Go - Added comprehensive anti-patterns section - Preserved existing MCP integration and multi-language support - Hardened Sondera install/test scripts (from code review findings): - Added placeholder URL validation that fails fast if repo URL unchanged - Pinned Sondera repo to versioned tag via SONDERA_VERSION env var - Changed git clone to --branch --depth 1 for pinned shallow clones - Fixed test script to use consistent paths matching installer Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewer's GuideEnriches the sc-test Claude skill with a more structured, interactive, multi-phase test coverage workflow (imported from Ocelot) and hardens the Sondera install/test shell scripts with safer defaults, version pinning, and consistent paths. Sequence diagram for enriched sc-test interactive coverage workflowsequenceDiagram
actor User
participant Claude_sc_test_skill
participant Codebase
User->>Claude_sc_test_skill: /sc:test --generate --target 80
Claude_sc_test_skill->>Codebase: Scan files and existing tests
Codebase-->>Claude_sc_test_skill: Current coverage and test inventory
%% Phase 1.4: Priority confirmation
Claude_sc_test_skill->>User: AskUserQuestion priority_choice
User-->>Claude_sc_test_skill: Choose highest_impact | quick_wins | specific_module | dry_run
%% Phase 2: Pattern analysis
Claude_sc_test_skill->>Codebase: Read test_infra and examples
Codebase-->>Claude_sc_test_skill: Test patterns, fixtures, helpers
Claude_sc_test_skill->>Claude_sc_test_skill: Classify functions and gaps
%% Phase 3: Detailed templates
Claude_sc_test_skill->>User: Present Python_TS_Go AAA templates
Claude_sc_test_skill->>User: AskUserQuestion language_and_scope
User-->>Claude_sc_test_skill: Confirm language and files to focus
%% Phase 4 and 5.3: Validation and iterative loop
loop Coverage_iteration until user_stops
Claude_sc_test_skill->>Codebase: Propose or update tests
User->>Codebase: Apply suggested tests
Claude_sc_test_skill->>Codebase: Run new tests and partial suite
Codebase-->>Claude_sc_test_skill: Test results and failures
Claude_sc_test_skill->>User: Present failures and fixes
Claude_sc_test_skill->>User: AskUserQuestion continue_strategy
User-->>Claude_sc_test_skill: continue | stop | raise_target | switch_focus
Claude_sc_test_skill->>Claude_sc_test_skill: Adjust target or focus
end
%% Phase 6: Summary report
Claude_sc_test_skill->>Codebase: Compute before_after_coverage
Codebase-->>Claude_sc_test_skill: Coverage_delta and per_file_stats
Claude_sc_test_skill->>User: Summary report with verification commands
Flow diagram for hardened Sondera installer scriptflowchart TD
Start([Start install-with-sondera.sh]) --> LoadEnv[Load SONDERA_REPO default
and SONDERA_VERSION default]
LoadEnv --> CheckCLAUDE[Check CLAUDE.md exists]
CheckCLAUDE -->|missing| ErrorCLAUDE[Print error and exit 1]
CheckCLAUDE -->|present| ValidateRepo[Check SONDERA_REPO does not contain your-org]
ValidateRepo -->|contains your-org| ErrorRepo[Print placeholder URL error
and exit 1]
ValidateRepo -->|valid URL| ShowIntro[Print installer introduction and prerequisites]
ShowIntro --> CheckDir{Does SONDERA_DIR exist?}
CheckDir -->|yes| UpdateExisting[cd SONDERA_DIR
and git fetch --tags
and git checkout SONDERA_VERSION]
CheckDir -->|no| CloneNew[mkdir -p parent of SONDERA_DIR
and git clone --branch SONDERA_VERSION --depth 1
from SONDERA_REPO into SONDERA_DIR
and cd SONDERA_DIR]
UpdateExisting --> Continue[Continue remaining setup steps]
CloneNew --> Continue
Continue --> End([End script])
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe pull request updates the sc-test skill documentation to restructure around coverage-focused testing with a multi-phase workflow, while improving the Sondera installation and integration scripts through version pinning and parameterized configuration paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
install-with-sondera.sh, consider explicitly handling failures fromgit checkout "$SONDERA_VERSION"(e.g., when the tag/branch does not exist) so the user gets a clear error message instead of continuing with a partially updated install. - In
test-sondera-integration.sh, the suggested harness command uses$SONDERA_DIR/target/release/sondera-harness-serverwhile the build check uses a plaincargo buildwithout--release; aligning the build mode and the expected binary path would avoid confusion or missing binaries at runtime.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `install-with-sondera.sh`, consider explicitly handling failures from `git checkout "$SONDERA_VERSION"` (e.g., when the tag/branch does not exist) so the user gets a clear error message instead of continuing with a partially updated install.
- In `test-sondera-integration.sh`, the suggested harness command uses `$SONDERA_DIR/target/release/sondera-harness-server` while the build check uses a plain `cargo build` without `--release`; aligning the build mode and the expected binary path would avoid confusion or missing binaries at runtime.
## Individual Comments
### Comment 1
<location path="install-with-sondera.sh" line_range="101-104" />
<code_context>
echo " Updating existing installation..."
cd "$SONDERA_DIR"
- git pull
+ git fetch --tags
+ git checkout "$SONDERA_VERSION"
else
- echo " Cloning repository..."
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Updating an existing install only fetches tags, which may not cover branch-based SONDERA_VERSION values.
Since the update path only runs `git fetch --tags` before `git checkout "$SONDERA_VERSION"`, branch values for SONDERA_VERSION (e.g., `main` or a release branch) may not be updated. Consider also fetching from the remote (e.g., `git fetch origin "$SONDERA_VERSION" --tags` or `git fetch origin`) so both tags and branches are current before checkout.
```suggestion
echo " Updating existing installation..."
cd "$SONDERA_DIR"
git fetch origin --tags
git checkout "$SONDERA_VERSION"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| echo " Updating existing installation..." | ||
| cd "$SONDERA_DIR" | ||
| git pull | ||
| git fetch --tags | ||
| git checkout "$SONDERA_VERSION" |
There was a problem hiding this comment.
suggestion (bug_risk): Updating an existing install only fetches tags, which may not cover branch-based SONDERA_VERSION values.
Since the update path only runs git fetch --tags before git checkout "$SONDERA_VERSION", branch values for SONDERA_VERSION (e.g., main or a release branch) may not be updated. Consider also fetching from the remote (e.g., git fetch origin "$SONDERA_VERSION" --tags or git fetch origin) so both tags and branches are current before checkout.
| echo " Updating existing installation..." | |
| cd "$SONDERA_DIR" | |
| git pull | |
| git fetch --tags | |
| git checkout "$SONDERA_VERSION" | |
| echo " Updating existing installation..." | |
| cd "$SONDERA_DIR" | |
| git fetch origin --tags | |
| git checkout "$SONDERA_VERSION" |
Claude Code Review (via AWS Bedrock)OverviewThis PR enriches the Critical Issues1. Supply Chain Security: Missing Integrity Verification
2. Environment Variable Injection Risk
High Priority3. Test Generation: Command Injection in Examples
4. Missing Error Handling in Version Pinning
5. Path Traversal Risk in Module Selection
Medium Priority6. Inconsistent Log Path Documentation
7. Hardcoded Timeout Values
8. Test Generation Anti-Pattern: Too Prescriptive
9. Missing Rollback Mechanism
10. Race Condition in Socket Check
Positive Observations✅ Excellent Documentation: ✅ Version Pinning Strategy: Adding ✅ Placeholder Validation: The check for "your-org" in repo URL (lines 29-35) prevents common misconfiguration errors ✅ Interactive Workflow Design: The ✅ Multi-Language Support: Comprehensive test templates for Python, TypeScript, and Go demonstrate real cross-ecosystem thinking ✅ KISS Principle Enforcement: Anti-patterns section explicitly discourages over-engineering test infrastructure ✅ Let It Crash Philosophy: Test examples correctly avoid defensive error handling, promoting cleaner test code ✅ Phase-Based Architecture: The 6-phase workflow (baseline → patterns → generate → validate → verify → iterate) is well-structured and maintainable ✅ Path Consistency: Centralizing ✅ Shallow Clone Optimization: Using Architecture & Design NotesTest Skill Architecture (⭐ Notable)
This follows the Single Responsibility Principle well. Each phase has clear inputs/outputs. Potential Concern: The 5-iteration hard limit (line 500) may frustrate users on large codebases. Consider making this configurable via flag. Security Hardening Evolution
The progression is sensible, though v3 capabilities would be valuable before wider adoption. Testing Coverage AnalysisTests Modified: 0 files Recommendation: Add shellcheck validation and basic test cases: # .github/workflows/shellcheck.yml
- name: Validate shell scripts
run: |
shellcheck install-with-sondera.sh test-sondera-integration.sh
- name: Test placeholder detection
run: |
export SONDERA_REPO="https://github.com/your-org/repo"
if ./install-with-sondera.sh; then
echo "FAIL: Should have rejected placeholder URL"
exit 1
fiReview Summary
Overall Assessment: This is a substantial quality improvement to test coverage capabilities with thoughtful security hardening. The critical issues around supply chain security should be addressed before merging if Sondera will be widely deployed. The test skill enhancements are production-ready. Recommendation:
Generated by Claude Code Review (AWS Bedrock) - Sonnet 4.5 |
|
See full review in next comment - PAL MCP Consensus Code Review (Manual Analysis) |
|
OVERVIEW: This PR enriches the sc-test skill with Ocelot comprehensive test coverage workflow and hardens the Sondera security layer installation scripts. Four files changed: new import documentation, major skill enhancement (240→584 lines), and security improvements to both Sondera shell scripts. POSITIVE OBSERVATIONS - Security Hardening (Excellent):
|
|
POSITIVE OBSERVATIONS - Code Quality (Strong):
Architecture (Well-Designed):
|
|
MEDIUM PRIORITY RECOMMENDATIONS:
|
|
LOW PRIORITY / NICE-TO-HAVE:
CRITICAL ISSUES: None Found REVIEW SUMMARY:
Overall Assessment: APPROVE - High-quality PR with significant security improvements. The recommendations above are advisory and non-blocking. The version pinning and placeholder validation are particularly valuable security enhancements. This review was performed manually following PAL MCP Consensus Code Review methodology. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.claude/skills/sc-test/SKILL.md (2)
219-233: Add language specifier to fenced code block.Per markdownlint MD040, fenced code blocks should have a language specified. This AskUserQuestion block could use
yamlsince the structure resembles YAML.📝 Suggested fix
-``` +```yaml AskUserQuestion: question: "Found <N> files below target. Which should I tackle first?"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/sc-test/SKILL.md around lines 219 - 233, The fenced code block containing the AskUserQuestion YAML should include a language specifier to satisfy markdownlint MD040; update the block that starts with "AskUserQuestion:" to use ```yaml instead of ``` so the yaml structure (question, header, multiSelect, options) is highlighted and linting passes.
273-279: Additional code blocks missing language specifiers.Similar to line 219, the code blocks at lines 273, 484, and 513 should have language specifiers (e.g.,
yamlfor AskUserQuestion blocks,markdownfor report templates).Also applies to: 484-498, 513-541
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/sc-test/SKILL.md around lines 273 - 279, Several fenced code blocks in SKILL.md (notably the fixture/helper table starting with "| Fixture/Helper | Purpose | Used By |", the AskUserQuestion examples, and the report template blocks) are missing language specifiers; update each backtick-fenced block to include an appropriate language tag (e.g., ```markdown for the table and report templates, ```yaml for AskUserQuestion examples) so syntax highlighting and linting work correctly, ensuring you change the opening fences only and preserve the existing block content and indentation.install-with-sondera.sh (2)
100-110: Consider handling dirty working directory state on update.If the local repository has uncommitted changes or conflicts,
git checkout "$SONDERA_VERSION"may fail or produce unexpected behavior. A safer approach would reset to the target version.🔧 Suggested safer update approach
if [ -d "$SONDERA_DIR" ]; then echo " Updating existing installation..." cd "$SONDERA_DIR" git fetch --tags - git checkout "$SONDERA_VERSION" + git checkout --force "$SONDERA_VERSION" elseAlternatively, use
git reset --hard "$SONDERA_VERSION"after fetch if you want to discard any local modifications.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install-with-sondera.sh` around lines 100 - 110, The update path that runs git fetch and git checkout "$SONDERA_VERSION" can fail if the working tree is dirty; modify the update branch that checks SONDERA_DIR to ensure a deterministic reset: after git fetch --tags, run a hard reset to the desired ref (e.g., git reset --hard "$SONDERA_VERSION" or checkout + git reset --hard) so local uncommitted changes are discarded and the repo is pinned to SONDERA_VERSION; update the block that references SONDERA_DIR and SONDERA_VERSION to perform this safe reset instead of a plain git checkout.
13-16: Consider makingSONDERA_DIRandSOCKET_PATHoverridable for consistency.
SONDERA_REPOandSONDERA_VERSIONuse parameter expansion to allow environment overrides, butSONDERA_DIRandSOCKET_PATHare hardcoded. This inconsistency may cause issues if users need to customize installation paths.♻️ Suggested change for consistency
SONDERA_REPO="${SONDERA_REPO:-https://github.com/your-org/sondera-coding-agent-hooks}" SONDERA_VERSION="${SONDERA_VERSION:-v0.1.0}" # Pin to a specific release tag -SONDERA_DIR="$HOME/.local/share/sondera-coding-agent-hooks" -SOCKET_PATH="/tmp/sondera-harness.sock" +SONDERA_DIR="${SONDERA_DIR:-$HOME/.local/share/sondera-coding-agent-hooks}" +SOCKET_PATH="${SOCKET_PATH:-/tmp/sondera-harness.sock}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install-with-sondera.sh` around lines 13 - 16, SONDERA_DIR and SOCKET_PATH are hardcoded but should be overridable like SONDERA_REPO/SONDERA_VERSION; change their assignments to use parameter expansion (e.g., SONDERA_DIR="${SONDERA_DIR:-$HOME/.local/share/sondera-coding-agent-hooks}" and SOCKET_PATH="${SOCKET_PATH:-/tmp/sondera-harness.sock}") so environment variables can override them, and update any subsequent references to use these variables consistently (e.g., in install, start, and cleanup steps).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test-sondera-integration.sh`:
- Line 67: Update the echoed log path in test-sondera-integration.sh so it
matches the install script's output location (/tmp/sondera-harness.log);
specifically change the string " - Harness logs: tail -f
$HOME/.local/state/sondera/harness.log" to reference "/tmp/sondera-harness.log"
(and search for any other occurrences of $HOME/.local/state/sondera/harness.log
to update them as well) so users can find the correct harness log written by the
install script.
---
Nitpick comments:
In @.claude/skills/sc-test/SKILL.md:
- Around line 219-233: The fenced code block containing the AskUserQuestion YAML
should include a language specifier to satisfy markdownlint MD040; update the
block that starts with "AskUserQuestion:" to use ```yaml instead of ``` so the
yaml structure (question, header, multiSelect, options) is highlighted and
linting passes.
- Around line 273-279: Several fenced code blocks in SKILL.md (notably the
fixture/helper table starting with "| Fixture/Helper | Purpose | Used By |", the
AskUserQuestion examples, and the report template blocks) are missing language
specifiers; update each backtick-fenced block to include an appropriate language
tag (e.g., ```markdown for the table and report templates, ```yaml for
AskUserQuestion examples) so syntax highlighting and linting work correctly,
ensuring you change the opening fences only and preserve the existing block
content and indentation.
In `@install-with-sondera.sh`:
- Around line 100-110: The update path that runs git fetch and git checkout
"$SONDERA_VERSION" can fail if the working tree is dirty; modify the update
branch that checks SONDERA_DIR to ensure a deterministic reset: after git fetch
--tags, run a hard reset to the desired ref (e.g., git reset --hard
"$SONDERA_VERSION" or checkout + git reset --hard) so local uncommitted changes
are discarded and the repo is pinned to SONDERA_VERSION; update the block that
references SONDERA_DIR and SONDERA_VERSION to perform this safe reset instead of
a plain git checkout.
- Around line 13-16: SONDERA_DIR and SOCKET_PATH are hardcoded but should be
overridable like SONDERA_REPO/SONDERA_VERSION; change their assignments to use
parameter expansion (e.g.,
SONDERA_DIR="${SONDERA_DIR:-$HOME/.local/share/sondera-coding-agent-hooks}" and
SOCKET_PATH="${SOCKET_PATH:-/tmp/sondera-harness.sock}") so environment
variables can override them, and update any subsequent references to use these
variables consistently (e.g., in install, start, and cleanup steps).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af10a5c4-d7c0-426c-88e4-7611c13f96be
📒 Files selected for processing (4)
.claude/skills/sc-test/IMPORT_NOTES.md.claude/skills/sc-test/SKILL.mdinstall-with-sondera.shtest-sondera-integration.sh
| echo "" | ||
| echo "Monitoring:" | ||
| echo " - Harness logs: tail -f /tmp/sondera-harness.log" | ||
| echo " - Harness logs: tail -f $HOME/.local/state/sondera/harness.log" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify log path references in both scripts
echo "=== install-with-sondera.sh log references ==="
rg -n "harness.log" install-with-sondera.sh
echo ""
echo "=== test-sondera-integration.sh log references ==="
rg -n "harness.log" test-sondera-integration.shRepository: Tony363/SuperClaude
Length of output: 478
Update log path to match install script.
The test output references $HOME/.local/state/sondera/harness.log, but the install script writes to /tmp/sondera-harness.log (lines 210, 232). Users will not find logs at the expected location.
Fix
-echo " - Harness logs: tail -f $HOME/.local/state/sondera/harness.log"
+echo " - Harness logs: tail -f /tmp/sondera-harness.log"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo " - Harness logs: tail -f $HOME/.local/state/sondera/harness.log" | |
| echo " - Harness logs: tail -f /tmp/sondera-harness.log" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test-sondera-integration.sh` at line 67, Update the echoed log path in
test-sondera-integration.sh so it matches the install script's output location
(/tmp/sondera-harness.log); specifically change the string " - Harness logs:
tail -f $HOME/.local/state/sondera/harness.log" to reference
"/tmp/sondera-harness.log" (and search for any other occurrences of
$HOME/.local/state/sondera/harness.log to update them as well) so users can find
the correct harness log written by the install script.
1 participant
Summary
sc-testskill with Ocelot's comprehensive test-coverage workflow, adding interactive decision points, detailed test writing templates (Python/TS/Go), iterative coverage loops, and pattern analysis phases — while preserving existing MCP integration and multi-language supportsc-test Enrichment (from Ocelot import)
AskUserQuestionat 3 decision pointsSondera Script Hardening (from code review)
install-with-sondera.shSONDERA_REPOcontainsyour-orginstall-with-sondera.shgit clone --branch $SONDERA_VERSION --depth 1test-sondera-integration.sh$SONDERA_DIRand$SOCKET_PATHvariablesTest plan
sc-testskill triggers correctly via/sc:test --generate --target 80install-with-sondera.shexits with error on placeholder URLtest-sondera-integration.shreferences match installer paths🤖 Generated with Claude Code
Summary by Sourcery
Enrich the sc-test skill documentation with an interactive, multi-phase coverage improvement workflow and harden Sondera install/test scripts for safer, version-pinned usage.
New Features:
Bug Fixes:
Enhancements:
Chores:
Summary by CodeRabbit
Documentation
Chores