Skip to content

test(sdk): add slash install smoke matrix#229

Open
Gradata wants to merge 1 commit into
mainfrom
gra-1680-slash-install-smoke
Open

test(sdk): add slash install smoke matrix#229
Gradata wants to merge 1 commit into
mainfrom
gra-1680-slash-install-smoke

Conversation

@Gradata
Copy link
Copy Markdown
Owner

@Gradata Gradata commented May 26, 2026

Summary

  • add a mocked-home smoke matrix for gradata hooks install --ide covering Claude Code and Codex
  • exercise the legacy gradata-install wrapper routing for Claude Code and Codex
  • explicitly track Hermes/OpenCode as skipped for the Show HN slash-command path with a GRA-1680 reason

Test

python3 -m pytest -q -rs tests/test_install_smoke_matrix.py tests/test_cli_install_agent.py
..ss..ss.....                                                            [100%]
=========================== short test summary info ============================
SKIPPED [2] tests/test_install_smoke_matrix.py:41: GRA-1680: legacy slash-command/one-command installer only accepts claude-code and codex before Show HN; use `gradata install --agent <host>` for this host until it is promoted into the slash-command path.
SKIPPED [2] tests/test_install_smoke_matrix.py:109: GRA-1680: legacy slash-command/one-command installer only accepts claude-code and codex before Show HN; use `gradata install --agent <host>` for this host until it is promoted into the slash-command path.
9 passed, 4 skipped in 1.54s

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Review Change Stack

📝 Walkthrough
  • Adds --ide flag to gradata hooks install command with support for claude-code and codex
  • Routes non-Claude Code IDE requests to agent-based installer by delegating to _cmd_install_agent
  • Introduces comprehensive smoke test matrix in test_install_smoke_matrix.py with 184 lines of test code
  • Tests IDE-specific hook configuration file creation and content verification
  • Exercises legacy gradata-install wrapper routing for supported IDEs
  • Explicitly skips Hermes/OpenCode for slash-command path (reason: GRA-1680)
  • Test results: 9 passed, 4 skipped
  • No breaking changes or public API modifications

Walkthrough

This PR introduces IDE/agent selection to the Gradata hooks installer CLI. The gradata hooks install command now accepts an --ide flag to choose between hook wiring adapters. Non-claude-code IDEs route through the agent-based installer, while claude-code preserves the existing implementation. Tests validate correct IDE-specific config generation and wrapper routing behavior.

Changes

IDE-aware hooks installer

Layer / File(s) Summary
CLI flag and install routing
Gradata/src/gradata/cli.py
The hooks subcommand adds --ide argument (choices: claude-code, codex; default: claude-code). The install action checks the IDE value and routes non-claude-code requests to the adapter-based agent installer via _cmd_install_agent, while claude-code uses the existing hook installation path.
Smoke test suite for IDE routing and config generation
Gradata/tests/test_install_smoke_matrix.py
Parametrized smoke tests verify Python CLI generates correct IDE-specific config files with expected content and asset validation. A second test suite (skipped if node is unavailable) validates that the JS installer wrapper routes correctly to gradata hooks install with the expected IDE argument. Test helpers manage subprocess environments and stub executables.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

feature

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'test(sdk): add slash install smoke matrix' accurately describes the main change—adding smoke tests for the slash install feature with IDE support.
Description check ✅ Passed The description clearly relates to the changeset, detailing the addition of smoke tests for gradata hooks install --ide, the legacy wrapper routing, and explicit skip tracking with reasoning.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gra-1680-slash-install-smoke

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.22.0)

OpenGrep fatal error (exit code 2):
┌──────────────┐
│ Opengrep CLI │
└──────────────┘

�[32m✔�[39m �[1mOpengrep OSS�[0m
�[32m✔�[39m Basic security coverage for first-party code vulnerabilities.

�[1m Loading rules from local config...�[0m
[00.20][ERROR]: Error: exception Glob.Lexer.Syntax_error("malformed glob pattern: missing ']'")
Raised at Glob__Lexer.syntax_error in file "libs/glob/Lexer.mll", line 8, characters 2-26
Called from Glob__Lexer.__ocaml_lex_token_rec in file "libs/glob/Lexer.mll", line 29, characters 26-53
Cal


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added the feature label May 26, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Gradata/src/gradata/cli.py`:
- Around line 1846-1855: The CLI currently accepts --ide for all hooks actions
but only applies it for install; update the hooks command handling so that
args.ide is either respected or rejected consistently: inside the hooks command
dispatcher (where args.ide is read and where _cmd_install_agent(...) is called),
detect non-install actions (e.g., uninstall, status) and either map args.ide to
args.agent for those actions or raise/print an error informing the user that
--ide is only valid with the install subcommand; specifically modify the block
using getattr(args, "ide", "claude-code") and the dispatcher that handles hooks
to refuse or handle --ide for non-install commands instead of silently ignoring
it, ensuring tests and help messages reflect the rule.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1f5dce8c-4261-4650-a29a-c87cfe543385

📥 Commits

Reviewing files that changed from the base of the PR and between a197bff and 5f1f1ca.

📒 Files selected for processing (2)
  • Gradata/src/gradata/cli.py
  • Gradata/tests/test_install_smoke_matrix.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest (py3.12)
  • GitHub Check: pytest (py3.11)
🧰 Additional context used
📓 Path-based instructions (2)
Gradata/src/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/src/**/*.py: Prefer sentence-transformers for local embeddings, google-genai for Gemini embeddings, cryptography for AES-GCM encrypted system.db, bm25s for BM25 rule ranking, and mem0ai for external memory adapters — guard all optional dependency imports with try / except ImportError at the call site, never at module level
Maintain strict layering: Layer 0 (Primitives: _types.py, _db.py, _events.py, _paths.py, _file_lock.py; Patterns: contrib/patterns/) must never import from Layer 1 (Enhancements: enhancements/, rules/) or Layer 2 (Public API: brain.py, cli.py, daemon.py, mcp_server.py)
Never use bare except: pass — use typed exceptions or at minimum logger.warning(...) with exc_info=True to avoid silent failure in a memory product
Never import from out-of-scope sibling directories ../Sprites/ or ../Hausgem/ within gradata/* code — that is a layering bug
Never leak private-sibling paths into public docs/code — no references to ../Sprites/, ../Hausgem/, email addresses, OneDrive paths, or Sprites-specific examples from inside gradata/*
Use atomic-write helper when writing JSON files to prevent corruption from mid-write crashes

Files:

  • Gradata/src/gradata/cli.py
Gradata/tests/**/*.py

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Gradata/tests/**/*.py: Set BRAIN_DIR environment variable via tmp_path in conftest.py for test isolation — ensure _paths.py module cache refreshes when calling Brain.init() directly inside tests
Add unit tests in tests/test_*.py for every CI push without LLM calls (deterministic); mark integration tests with @pytest.mark.integration and skip them by default (they hit real LLM APIs)

Files:

  • Gradata/tests/test_install_smoke_matrix.py
🔇 Additional comments (1)
Gradata/tests/test_install_smoke_matrix.py (1)

17-31: LGTM!

Also applies to: 41-107, 109-180, 182-184

Comment on lines +1846 to +1855
ide = getattr(args, "ide", "claude-code")
if ide != "claude-code":
# The legacy one-command installer calls `gradata hooks install
# --ide <host>`. Claude Code keeps using the richer hook installer
# below; other hosts are wired through the adapter-based install
# path used by `gradata install --agent <host>`.
args.agent = ide
_cmd_install_agent(args)
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

--ide is globally accepted but ignored for non-install actions

Line 2170 adds --ide for all hooks actions, but Line 1846-1855 only applies it in install. hooks uninstall --ide codex is currently accepted and silently ignored, which is misleading and can cause users to think Codex uninstall/status was handled.

Proposed fix
 def cmd_hooks(args):
     """Manage Claude Code hook integration."""
     action = args.action
+    ide = getattr(args, "ide", "claude-code")
+    if action in ("uninstall", "status") and ide != "claude-code":
+        print(
+            "error: --ide is only supported for `hooks install`; "
+            "use `gradata uninstall --agent <host>` for non-claude hosts",
+            file=sys.stderr,
+        )
+        sys.exit(2)
+
     if action == "install":
-        ide = getattr(args, "ide", "claude-code")
         if ide != "claude-code":
             # The legacy one-command installer calls `gradata hooks install
             # --ide <host>`. Claude Code keeps using the richer hook installer
             # below; other hosts are wired through the adapter-based install
             # path used by `gradata install --agent <host>`.
             args.agent = ide
             _cmd_install_agent(args)
             return

Also applies to: 2170-2175

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Gradata/src/gradata/cli.py` around lines 1846 - 1855, The CLI currently
accepts --ide for all hooks actions but only applies it for install; update the
hooks command handling so that args.ide is either respected or rejected
consistently: inside the hooks command dispatcher (where args.ide is read and
where _cmd_install_agent(...) is called), detect non-install actions (e.g.,
uninstall, status) and either map args.ide to args.agent for those actions or
raise/print an error informing the user that --ide is only valid with the
install subcommand; specifically modify the block using getattr(args, "ide",
"claude-code") and the dispatcher that handles hooks to refuse or handle --ide
for non-install commands instead of silently ignoring it, ensuring tests and
help messages reflect the rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant