feat(hogli): add ci:insights adapter (with Mendral concrete class)#60299
Conversation
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
tools/hogli-commands/hogli_commands/tests/test_ci_insights.py:45-53
The parametrize table covers `view --json` but omits the analogous `search --json` case, leaving the `--json` flag on `search` untested despite the project preference for parameterised tests covering symmetric behaviour.
```suggestion
@pytest.mark.parametrize(
"argv, expected",
[
([], ("mendral", "here")),
(["search", "flaky timeout"], ("mendral", "insight", "search", "flaky timeout")),
(["search", "flaky timeout", "--json"], ("mendral", "insight", "search", "flaky timeout", "--json")),
(["view", "01ABC"], ("mendral", "insight", "view", "01ABC")),
(["view", "01ABC", "--json"], ("mendral", "insight", "view", "01ABC", "--json")),
],
)
```
### Issue 2 of 2
tools/hogli-commands/hogli_commands/tests/test_ci_insights.py:106
The payload `'{"actions": [null]}'` is valid JSON, so it never reaches the `json.JSONDecodeError` handler — the null entry is silently filtered and the code lands on the "No remediation plan available" path, which is already the focus of `test_plan_errors_when_no_actions`. Keeping it here conflates two distinct failure modes; moving it there as a second parametrize case keeps the intent explicit.
```suggestion
@pytest.mark.parametrize("payload", ["", "not json{"])
```
Reviews (1): Last reviewed commit: "fix(hogli): harden ci:insights edge case..." | Re-trigger Greptile |
There was a problem hiding this comment.
The hex-security-app bot raised a valid, current-head, unresolved argument-injection concern: query and insight_id are passed directly as positional elements in the subprocess.run args list with no -- option terminator, so a value like --config=... or --token=... would be interpreted as a flag by the mendral CLI rather than as a literal argument. The fix is a one-line change (place "--" before each user-supplied positional in search, view, and plan). Per review policy, substantive bot comments with valid concerns that remain unaddressed are a REFUSE.
There was a problem hiding this comment.
The hex-security-app bot flagged argument injection in search, view, and plan — user-supplied query and insight_id are passed as positional args to subprocess.run without a -- terminator, so a value starting with - would be interpreted as a flag by the mendral CLI. The thread was marked resolved but the code was not changed, and the stamphog aggregator top-level review (current head) explicitly calls this out as still unaddressed. The fix is trivial — add "--" before each user-supplied positional — and should be applied before auto-approving.
|
|
||
|
|
||
| class MendralBackend: | ||
| """The only Mendral-aware code. Swap this class to change providers.""" |
Problem
Mendral (our CI-insights backend) holds aggregated cross-run intelligence — recurring flakes, occurrence counts, confidence, proposed/merged fixes — that no single CI run can show. Today it lives only in Slack and the web UI, so engineers (and agents) rarely consult it at the moment they're actually debugging a red build.
Changes
hogli ci:insightscommand — an adapter over the CI-insights backend. Verbs:ci:insights(digest for the current repo + branch, default),search,view [--json],plan(prints the recommended remediation plan; does not apply it). All Mendral-specific knowledge is quarantined in oneMendralBackendclass. Lives in thehogli-commandsextension layer; core untouched.debugging-ci-failuresskill gains a "Check existing CI insights first" step (between Classification and Local reproduction) so agents query an existing hypothesis before reproducing from scratch.How did you test this code?
I'm an agent; I did not perform manual QA beyond the runs below. Automated + live checks I actually ran:
test_ci_insights.py— 16 passing (preflight, verb→backend mapping, exit-code propagation, recommended-action selection).hogli ci:insights/search/plan <id>against the live backend;plancorrectly selected aproposedrecommended action over amergedone; bad id exits non-zero.ruff check/formatclean;hogli lint:skills+build:skillspass with no file drift.Publish to changelog?
No.
🤖 Agent context
Authored with Claude Code