Skip to content

feat(cli): flag-gated per-CLI install verification for install --agent#189

Merged
Gradata merged 4 commits into
mainfrom
feat/gra-214-install-verification
May 13, 2026
Merged

feat(cli): flag-gated per-CLI install verification for install --agent#189
Gradata merged 4 commits into
mainfrom
feat/gra-214-install-verification

Conversation

@Gradata

@Gradata Gradata commented May 13, 2026

Copy link
Copy Markdown
Owner

Summary

Add GRADATA_VERIFY_INSTALL env flag (1/true/yes) to _cmd_install_agent. When set, after each successful agent install, writes a test rule via Brain.correct() then reads it back via Brain.search(). Failures surface explicit actionable errors. Flag OFF preserves current behavior.

Test plan

5 tests pass in tests/test_cli_install_agent.py

Flag details

  • Name: GRADATA_VERIFY_INSTALL
  • Default: OFF
  • Rollback: unset env var

Add GRADATA_VERIFY_INSTALL env flag. When set (1/true/yes),
_cmd_install_agent writes then reads a test rule via Brain.correct()
+ Brain.search() after each successful agent install. Failures
surface explicit actionable errors. Flag OFF preserves current behavior.

Tests (5 pass):
- test_cli_install_agent_writes_config_under_fake_home
- test_cli_install_agent_all_detects_existing_configs
- test_cli_install_agent_verify_flag_off_preserves_current_behavior
- test_cli_install_agent_verify_flag_on_shows_confirmation
- test_cli_install_agent_verify_flag_skips_on_failed_install

@greptile-apps greptile-apps 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.

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

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Install verification feature: Added GRADATA_VERIFY_INSTALL environment variable flag to enable post-install verification for install --agent command; when enabled, writes a test rule and confirms readback to verify install correctness
Non-persistent verification: Test corrections are written to a temporary/scratch brain instead of the user's memory DB to avoid polluting metrics or exports
Deterministic search: Verification uses full unique token (including random suffix) for precise readback to avoid matching old artifacts
Opt-in with backward compatibility: Flag defaults to OFF; existing behavior is preserved when unset; rollback via unsetting the environment variable
Test coverage: Added 3 new integration tests covering flag-off, flag-on success, and failed-install scenarios
Test hermiticity: Updated _run_cli() test helper to accept optional env mapping and explicitly unset GRADATA_* environment variables to prevent host environment pollution of tests
Dev dependency: Added httpx>=0.27.0 to dev extras for CI test stability
No public API changes: Implementation is internal to CLI install flow

Walkthrough

The CLI's agent install flow now optionally runs a post-install verification when GRADATA_VERIFY_INSTALL is enabled: it writes a temporary test correction via Brain, searches for a unique marker to confirm readback, and treats verification failures or exceptions as install failures. Tests and a dev dependency were added to exercise and support this behavior.

Changes

Install Verification Feature

Layer / File(s) Summary
Install verification implementation
Gradata/src/gradata/cli.py
Adds GRADATA_VERIFY_INSTALL gate and implements post-install verification that writes a temporary correction, searches for the unique marker, and marks the install as failed if verification fails or raises.
Test infrastructure and verification behavior tests
Gradata/tests/test_cli_install_agent.py, Gradata/pyproject.toml
Updates _run_cli to accept env overrides and sanitize existing GRADATA_ keys; adds three tests covering verify-off, verify-on (successful), and verify-on with failed install (skipped). Adds httpx>=0.27.0 to dev dependencies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 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 clearly and specifically describes the main change: adding a flag-gated verification feature for CLI agent installation.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the new GRADATA_VERIFY_INSTALL flag, its behavior, test coverage, and rollback mechanism.
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 feat/gra-214-install-verification

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.20.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.34][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 13, 2026

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 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 320-323: The install verification is persisting synthetic
corrections by calling brain.correct(...) (lines ~320-333); change this to a
non-persistent verification flow or ensure explicit cleanup. Replace the
persistent call to brain.correct with a transient API (e.g., a
non-persistent/ephemeral verification method) if one exists, or if not, capture
the returned correction identifier and delete it immediately in a finally block
(use brain.delete_correction/delete_rule/forget or equivalent) so no synthetic
draft/final data remains in user memory after the install check. Ensure the fix
references the brain.correct invocation and the corresponding cleanup call so
re-running installs cannot accumulate fake rules/events.
- Around line 324-329: The current verify search uses a static key
"gradata-install-verify-{name}" which can match older artifacts; change the
write to include a unique marker (e.g., timestamp or UUID) and use that exact
marker for the readback so you prove the same record was written and read.
Concretely: when writing the test rule, append a unique_token (uuid4 or
time-based string) to the rule content/key (store it in a local variable like
unique_token), then replace the brain.search call with a lookup/search for
f"gradata-install-verify-{name}-{unique_token}" (or fetch by returned record id
if write returns one) and assert the returned result contains that unique_token;
update variables/results/had_failure handling accordingly.

In `@Gradata/tests/test_cli_install_agent.py`:
- Around line 10-17: The test helper builds base_env using os.environ.copy()
which leaks host feature flags (e.g., GRADATA_VERIFY_INSTALL) into tests; modify
the helper that sets base_env so it either starts from a minimal clean env (not
os.environ.copy()) or explicitly removes/unsets any GRADATA_* and related
feature flags (specifically ensure GRADATA_VERIFY_INSTALL is not present) before
returning; update the logic around the base_env variable creation/update (the
code that sets HOME, USERPROFILE, XDG_CONFIG_HOME, PYTHONPATH and then updates
with env) to guarantee deterministic “flag off” behavior.
🪄 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: 0a2c488f-78aa-488c-b01f-4d1e5bb847a2

📥 Commits

Reviewing files that changed from the base of the PR and between 5a5c4d8 and 1a9d2b7.

📒 Files selected for processing (2)
  • Gradata/src/gradata/cli.py
  • Gradata/tests/test_cli_install_agent.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 macos-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.11
  • GitHub Check: pytest ubuntu-latest / py3.12
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest (py3.11)
  • GitHub Check: pytest (py3.12)
🧰 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_cli_install_agent.py

Comment thread Gradata/src/gradata/cli.py Outdated
Comment thread Gradata/src/gradata/cli.py Outdated
Comment thread Gradata/tests/test_cli_install_agent.py
@Gradata

Gradata commented May 13, 2026

Copy link
Copy Markdown
Owner Author

Boss review (GRA-214)

Good feature direction. Per-CLI verification gates a real reliability gap. Three issues to fix before merge:

1. Synthetic correction persists in user memory (blocking)

brain.correct() writes a real correction that the user never authored. This pollutes their rule DB, inflates graduation metrics, and can leak into AGENTS.md exports. If Brain has a delete/forget API, call it in a finally block after the search succeeds. If not, either (a) add a transient/ephemeral flag to correct(), or (b) track the returned correction ID and delete immediately. Accumulating fake rules on re-runs is a non-starter for a memory product.

2. Verify search should use the exact unique token for readback

The search uses just gradata-install-verify-{name} which catches all old artifacts with that prefix. The random suffix (os.urandom(4).hex()) is fine for uniqueness, but the readback should search for the FULL token including the random suffix. Otherwise you're proving "something exists" not "the exact thing I just wrote exists." Minor but sloppy.

3. Test helper leaks host env vars (blocking)

_run_cli does os.environ.copy() — if GRADATA_VERIFY_INSTALL is set on the host, the "flag off" test fails nondeterministically. Explicitly unset GRADATA_VERIFY_INSTALL (and any other GRADATA_* flags) in base_env before returning. Tests must be hermetic.

Verdict

Fix items 1 and 3, tighten item 2, re-request review. This is close — the core logic is sound and the test coverage is good.

@greptile-apps greptile-apps 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.

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

@greptile-apps greptile-apps 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.

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

@greptile-apps greptile-apps 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.

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

@coderabbitai coderabbitai 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.

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/pyproject.toml`:
- Line 58: Remove the duplicate dependency entry "httpx>=0.27.0" from the dev
extras so it is only declared once under the llm optional group; locate the dev
extras block in the pyproject toml and delete the "httpx>=0.27.0" line (the llm
= ["httpx>=0.27.0"] entry should remain unchanged) to avoid redundant
declarations when installing --all-extras.
🪄 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: 571f8ea5-eb87-40bf-842a-16353bec49b4

📥 Commits

Reviewing files that changed from the base of the PR and between 1a9d2b7 and 039dc50.

⛔ Files ignored due to path filters (1)
  • Gradata/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Gradata/pyproject.toml
  • Gradata/src/gradata/cli.py
  • Gradata/tests/test_cli_install_agent.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.12
  • GitHub Check: pytest macos-latest / py3.11
  • GitHub Check: pytest ubuntu-latest / py3.11
  • GitHub Check: pytest windows-latest / py3.11
  • GitHub Check: pytest macos-latest / py3.12
  • GitHub Check: pytest windows-latest / py3.12
  • GitHub Check: pytest (py3.11)
  • GitHub Check: pytest (py3.12)
🧰 Additional context used
📓 Path-based instructions (3)
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_cli_install_agent.py
Gradata/**/pyproject.toml

📄 CodeRabbit inference engine (Gradata/AGENTS.md)

Maintain dependencies = [] in pyproject.toml — the base package is pure Python + stdlib with all heavy dependencies gated as optional extras: embeddings, gemini, encrypted, ranking, adapters-mem0

Files:

  • Gradata/pyproject.toml
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
🔇 Additional comments (2)
Gradata/src/gradata/cli.py (1)

294-297: LGTM!

Also applies to: 313-344

Gradata/tests/test_cli_install_agent.py (1)

9-23: LGTM!

Also applies to: 58-117

Comment thread Gradata/pyproject.toml
"bandit>=1.7",
"coverage>=7.0",
"ruff>=0.4",
"httpx>=0.27.0",

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 | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if tests import or use httpx directly

rg -n --type=py -C3 'import httpx|from httpx' tests/

Repository: Gradata/gradata

Length of output: 106


🏁 Script executed:

#!/bin/bash
# Find test directories and files
fd -type d -name 'test*' | head -20

Repository: Gradata/gradata

Length of output: 230


🏁 Script executed:

#!/bin/bash
# List repository structure
git ls-files | grep -E '(test|spec)' | head -20

Repository: Gradata/gradata

Length of output: 859


🏁 Script executed:

#!/bin/bash
# Read pyproject.toml to verify httpx placement
cat -n Gradata/pyproject.toml | sed -n '25p;30p;35p;40p;50p;58p;65p'

Repository: Gradata/gradata

Length of output: 446


🏁 Script executed:

#!/bin/bash
# Get context around line 58 in pyproject.toml
cat -n Gradata/pyproject.toml | sed -n '50,65p'

Repository: Gradata/gradata

Length of output: 435


🏁 Script executed:

#!/bin/bash
# Search for httpx usage in tests
rg 'import httpx|from httpx' Gradata/tests/ -n

Repository: Gradata/gradata

Length of output: 169


🏁 Script executed:

#!/bin/bash
# Also check if tests reference llm extra or call code that uses httpx
rg 'httpx' Gradata/tests/ -n -C 2

Repository: Gradata/gradata

Length of output: 1215


🏁 Script executed:

#!/bin/bash
# Read test_byo_key_provider.py to understand httpx usage context
cat -n Gradata/tests/test_byo_key_provider.py | head -40

Repository: Gradata/gradata

Length of output: 1633


🏁 Script executed:

#!/bin/bash
# Check if BYOKeyProvider is only in llm or also in base package
rg 'class BYOKeyProvider|from.*BYOKeyProvider|import.*BYOKeyProvider' Gradata/src/ -n

Repository: Gradata/gradata

Length of output: 457


🏁 Script executed:

#!/bin/bash
# Check if BYOKeyProvider could ever be imported or used without llm extra
# Look for any base/pure-python code that references BYOKeyProvider
rg 'BYOKeyProvider' Gradata/src/gradata --type=py | grep -v 'llm'

Repository: Gradata/gradata

Length of output: 169


🏁 Script executed:

#!/bin/bash
# Check how CI installs the package - does it use [dev,llm] or just [dev]?
cat Gradata/.github/workflows/test.yml | grep -A5 -B5 'pip install\|pyproject.toml'

Repository: Gradata/gradata

Length of output: 126


🏁 Script executed:

#!/bin/bash
# Find CI test workflow files
fd -name 'test.yml' -o -name 'test.yaml' 2>/dev/null

Repository: Gradata/gradata

Length of output: 41


🏁 Script executed:

#!/bin/bash
# Check test.yml location from earlier results
find . -path '*/.github/workflows/test.yml' 2>/dev/null

Repository: Gradata/gradata

Length of output: 88


🏁 Script executed:

#!/bin/bash
# Look at the test workflow file
cat -n .github/workflows/test.yml | grep -A10 -B5 'pip install\|extras'

Repository: Gradata/gradata

Length of output: 412


Remove httpx>=0.27.0 from dev extras—it's already declared in the llm extras.

httpx is already a dependency of the llm optional group (line 35). Since tests import BYOKeyProvider from gradata.llm and CI installs with --all-extras, this creates unnecessary duplication.

Current state
llm = ["httpx>=0.27.0"]  # line 35
dev = [
    "pytest>=8.0",
    ...
    "httpx>=0.27.0",  # line 58 — redundant
]

Remove line 58 to eliminate the duplication and reduce maintenance surface.

🤖 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/pyproject.toml` at line 58, Remove the duplicate dependency entry
"httpx>=0.27.0" from the dev extras so it is only declared once under the llm
optional group; locate the dev extras block in the pyproject toml and delete the
"httpx>=0.27.0" line (the llm = ["httpx>=0.27.0"] entry should remain unchanged)
to avoid redundant declarations when installing --all-extras.

@Gradata Gradata merged commit 708ce01 into main May 13, 2026
9 checks passed
@Gradata Gradata deleted the feat/gra-214-install-verification branch May 13, 2026 18:43
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