diff --git a/README.md b/README.md index c1640659..4dd51c47 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,7 @@ A single source might touch 10-15 wiki pages. Knowledge accumulates: each docume | `openkb lint` | Run structural + knowledge health checks | | `openkb list` | List indexed documents and concepts | | `openkb status` | Show knowledge base stats | +| openkb feedback ["msg"] | File feedback by opening a prefilled GitHub issue (use `--type bug/feature/question` to tag the issue) | diff --git a/openkb/cli.py b/openkb/cli.py index d574ef1e..5197a930 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -1133,3 +1133,154 @@ def status(ctx): click.echo("No knowledge base found. Run `openkb init` first.") return print_status(kb_dir) + + +# --------------------------------------------------------------------------- +# feedback +# --------------------------------------------------------------------------- + +_FEEDBACK_REPO = "VectifyAI/OpenKB" +_FEEDBACK_TYPES = ("bug", "feature", "question", "other") +_FEEDBACK_LABEL_MAP = { + "bug": "bug", + "feature": "enhancement", + "question": "question", + "other": "", +} + + +def _openkb_version() -> str: + """Return the installed openkb package version. + + Delegates to ``openkb.__version__`` so the chat REPL, feedback issue + body, and any future caller all surface the same fallback string + (``0.0.0+unknown`` from ``openkb/__init__.py``). Mirrors + ``openkb.agent.chat._openkb_version``. + """ + from openkb import __version__ + return __version__ + + +def _collect_feedback_diagnostics(ctx) -> dict[str, str]: + """Auto-collect non-sensitive environment info to attach to a feedback + issue. Kept deliberately small — no paths, no API keys, no usernames. + """ + import platform + + kb_dir = _find_kb_dir(ctx.obj.get("kb_dir_override") if ctx.obj else None) + return { + "openkb": _openkb_version(), + "python": platform.python_version(), + "platform": f"{platform.system()} {platform.release()}", + "kb_initialised": "yes" if kb_dir else "no", + } + + +def _build_feedback_url( + message: str, feedback_type: str, diagnostics: dict[str, str], +) -> str: + """Build a GitHub issue URL with title / body / labels prefilled.""" + from urllib.parse import urlencode + + first_line = message.splitlines()[0] if message else "" + truncated = first_line[:60] + ("…" if len(first_line) > 60 else "") + title_prefix = f"[{feedback_type}] " if feedback_type != "other" else "" + title = f"{title_prefix}{truncated}" if truncated else f"{title_prefix}Feedback from CLI" + + if diagnostics: + diag_block = "\n".join(f"- **{k}**: {v}" for k, v in diagnostics.items()) + body = ( + f"{message}\n\n" + "---\n\n" + "
\n" + "Diagnostics (auto-collected by openkb feedback)\n\n" + f"{diag_block}\n" + "
\n" + ) + else: + body = message + + params = {"title": title, "body": body} + label = _FEEDBACK_LABEL_MAP.get(feedback_type, "") + if label: + params["labels"] = label + + return f"https://github.com/{_FEEDBACK_REPO}/issues/new?{urlencode(params)}" + + +@cli.command() +@click.argument("message", required=False) +@click.option( + "--type", "feedback_type", + type=click.Choice(_FEEDBACK_TYPES), + default=None, + help="Feedback type — sets the GitHub issue label.", +) +@click.pass_context +def feedback(ctx, message, feedback_type): + """Submit feedback by opening a prefilled GitHub issue. + + Examples: + + \b + openkb feedback # interactive + openkb feedback "openkb add hangs on .docx" # one-line bug report + openkb feedback --type feature "..." # tags the issue 'enhancement' + + The command does not send anything to OpenKB maintainers directly — + it opens GitHub in your browser with title, body, and label prefilled. + You log in with your own GitHub account and submit the issue. + """ + if not message: + click.echo( + "What's your feedback? End with an empty line + Ctrl-D " + "(Unix) or Ctrl-Z+Enter (Windows). Ctrl-C cancels." + ) + message = sys.stdin.read().strip() + + if not message: + click.echo("No feedback provided. Aborted.") + ctx.exit(1) + return + + if feedback_type is None: + # Skip the prompt in non-TTY contexts (CI / piped stdin) so + # ``echo "msg" | openkb feedback`` doesn't hang on the second + # prompt after consuming all piped input for the message body. + # Mirrors the ``_stdin_is_tty()`` gate added in PR #48. + if _stdin_is_tty(): + feedback_type = click.prompt( + "Type", + default="other", + type=click.Choice(_FEEDBACK_TYPES), + show_default=True, + show_choices=True, + ) + else: + feedback_type = "other" + + diagnostics = _collect_feedback_diagnostics(ctx) + url = _build_feedback_url(message, feedback_type, diagnostics) + + click.echo("Copy this URL into a browser if the auto-open below fails:") + click.echo(f" {url}") + + import webbrowser + try: + opened = webbrowser.open(url) + except Exception as exc: + # webbrowser.open rarely raises but be defensive — the printed URL + # above is the fallback path. + click.echo(f" (browser auto-open failed: {exc})", err=True) + return + + # ``webbrowser.open`` returns False on headless boxes (no GUI, no + # ``BROWSER`` env) without raising. Without this check we'd silently + # print "Opened" and the user would think the issue was filed. + if opened: + click.echo("Opened GitHub in your browser.") + else: + click.echo( + " (no browser available — copy the URL above to file the issue)", + err=True, + ) diff --git a/tests/test_feedback.py b/tests/test_feedback.py new file mode 100644 index 00000000..96719de0 --- /dev/null +++ b/tests/test_feedback.py @@ -0,0 +1,242 @@ +"""Tests for `openkb feedback` — the prefilled-GitHub-issue feedback flow.""" +from __future__ import annotations + +from unittest.mock import patch +from urllib.parse import parse_qs, urlparse + +import pytest +from click.testing import CliRunner + +from openkb.cli import ( + _build_feedback_url, + _collect_feedback_diagnostics, + _FEEDBACK_REPO, + cli, +) + + +# --------------------------------------------------------------------------- +# _build_feedback_url +# --------------------------------------------------------------------------- + + +def _parse(url: str) -> dict: + """Parse a prefilled-issue URL into its query-param dict (single values).""" + parts = urlparse(url) + qs = parse_qs(parts.query) + # parse_qs yields lists; flatten the singletons we care about. + return {k: v[0] for k, v in qs.items()} + + +def test_build_url_points_at_correct_repo_issue_new(): + url = _build_feedback_url("hello", "bug", {}) + parts = urlparse(url) + assert parts.scheme == "https" + assert parts.netloc == "github.com" + assert parts.path == f"/{_FEEDBACK_REPO}/issues/new" + + +def test_build_url_title_includes_type_prefix(): + url = _build_feedback_url("attach fails on docx", "bug", {}) + params = _parse(url) + assert params["title"] == "[bug] attach fails on docx" + + +def test_build_url_title_omits_prefix_for_other_type(): + """'other' is the catch-all; don't pollute the title with [other].""" + url = _build_feedback_url("just a comment", "other", {}) + params = _parse(url) + assert params["title"] == "just a comment" + + +def test_build_url_title_truncated_at_60_chars(): + long_msg = "a" * 200 + url = _build_feedback_url(long_msg, "bug", {}) + params = _parse(url) + # 60 chars + ellipsis + prefix + assert params["title"] == "[bug] " + ("a" * 60) + "…" + + +def test_build_url_title_uses_first_line_only(): + """A multi-line message should only use line 1 for the title.""" + url = _build_feedback_url("short title\n\ndetailed body here", "feature", {}) + params = _parse(url) + assert params["title"] == "[feature] short title" + + +def test_build_url_label_set_for_bug(): + url = _build_feedback_url("x", "bug", {}) + params = _parse(url) + assert params["labels"] == "bug" + + +def test_build_url_label_mapped_for_feature(): + """Feature → 'enhancement' (GitHub's conventional label).""" + url = _build_feedback_url("x", "feature", {}) + params = _parse(url) + assert params["labels"] == "enhancement" + + +def test_build_url_no_label_for_other(): + url = _build_feedback_url("x", "other", {}) + params = _parse(url) + assert "labels" not in params + + +def test_build_url_diagnostics_attached_when_provided(): + url = _build_feedback_url( + "x", "bug", + {"openkb": "1.2.3", "python": "3.12.0", "platform": "Linux 6.0"}, + ) + params = _parse(url) + assert "Diagnostics" in params["body"] + assert "**openkb**: 1.2.3" in params["body"] + assert "**python**: 3.12.0" in params["body"] + assert "**platform**: Linux 6.0" in params["body"] + + +def test_build_url_no_diagnostics_block_when_empty(): + """When called with an empty dict the function omits the details block. + Defensive: the CLI always passes a populated dict, but keeping the + branch tested guards against accidental regression.""" + url = _build_feedback_url("just the message", "bug", {}) + params = _parse(url) + assert params["body"] == "just the message" + assert "Diagnostics" not in params["body"] + assert "
" not in params["body"] + + +# --------------------------------------------------------------------------- +# _collect_feedback_diagnostics +# --------------------------------------------------------------------------- + + +def test_collect_diagnostics_returns_minimal_non_sensitive_set(tmp_path): + """Diagnostics should be the small known set — no paths, no env vars.""" + + class _Ctx: + obj = None + + with patch("openkb.cli._find_kb_dir", return_value=None): + info = _collect_feedback_diagnostics(_Ctx()) + + assert set(info.keys()) == {"openkb", "python", "platform", "kb_initialised"} + assert info["kb_initialised"] == "no" + # Defensive: no path-like values that would leak the user's home dir. + for v in info.values(): + assert "/Users/" not in v + assert "/home/" not in v + + +def test_collect_diagnostics_flags_kb_present(tmp_path): + class _Ctx: + obj = None + + with patch("openkb.cli._find_kb_dir", return_value=tmp_path): + info = _collect_feedback_diagnostics(_Ctx()) + + assert info["kb_initialised"] == "yes" + + +# --------------------------------------------------------------------------- +# CLI: openkb feedback +# --------------------------------------------------------------------------- + + +def test_feedback_one_liner_opens_browser_with_url(): + """Default path: build URL, print it for copy-fallback, and open browser.""" + runner = CliRunner() + with patch("webbrowser.open") as mock_open: + result = runner.invoke(cli, ["feedback", "--type", "bug", "test message"]) + + assert result.exit_code == 0, result.output + mock_open.assert_called_once() + called_url = mock_open.call_args[0][0] + assert called_url.startswith("https://github.com/VectifyAI/OpenKB/issues/new?") + # The URL is also printed so the user has a copy if auto-open fails. + assert called_url in result.output + + +def test_feedback_empty_message_aborts_with_exit_1(): + """Interactive mode: if user submits nothing, abort cleanly (no issue URL).""" + runner = CliRunner() + # input="" simulates Ctrl-D on an empty stdin. + result = runner.invoke(cli, ["feedback", "--type", "bug"], input="") + assert result.exit_code == 1 + assert "No feedback provided" in result.output + + +def test_feedback_prompts_for_type_when_not_given_via_flag(): + """If --type isn't on the command line and stdin is a TTY, prompt the user.""" + runner = CliRunner() + with patch("webbrowser.open"), \ + patch("openkb.cli._stdin_is_tty", return_value=True): + result = runner.invoke( + cli, ["feedback", "missing-type prompt test"], + input="feature\n", + ) + + assert result.exit_code == 0 + # The URL printed for fallback-copy carries the chosen type's label. + url_line = [ln for ln in result.output.splitlines() if "issues/new" in ln][-1] + assert "labels=enhancement" in url_line + + +# --------------------------------------------------------------------------- +# Regressions from the self-review on PR #53 +# --------------------------------------------------------------------------- + + +def test_feedback_skips_type_prompt_when_stdin_is_not_a_tty(): + """In CI / piped contexts the second prompt would hang or abort + confusingly — the command must fall through to a default.""" + runner = CliRunner() + with patch("webbrowser.open"), \ + patch("openkb.cli._stdin_is_tty", return_value=False): + result = runner.invoke(cli, ["feedback", "non-tty feedback"]) + + assert result.exit_code == 0, result.output + # Non-TTY → falls back to "other", which has no label param + url_line = [ln for ln in result.output.splitlines() if "issues/new" in ln][-1] + assert "labels=" not in url_line + # And the title should NOT have a type prefix + assert "%5Bother%5D" not in url_line # urlencoded "[other]" + + +def test_feedback_warns_when_webbrowser_open_returns_false(): + """`webbrowser.open` returns False on headless boxes without raising — + the command must surface that to the user, not silently pretend + success.""" + runner = CliRunner() + with patch("webbrowser.open", return_value=False) as mock_open: + result = runner.invoke( + cli, ["feedback", "--type", "bug", "headless test"], + ) + + assert result.exit_code == 0, result.output + mock_open.assert_called_once() + # The success-confirmation message must NOT appear + assert "Opened GitHub in your browser" not in result.output + # The user must see a clear "no browser available" indication + assert "no browser available" in result.output + + +def test_feedback_confirms_when_webbrowser_open_succeeds(): + runner = CliRunner() + with patch("webbrowser.open", return_value=True): + result = runner.invoke( + cli, ["feedback", "--type", "bug", "happy path"], + ) + + assert result.exit_code == 0, result.output + assert "Opened GitHub in your browser" in result.output + + +def test_openkb_version_helper_matches_package_version(): + """`_openkb_version` in cli.py must delegate to `openkb.__version__` + so the chat REPL and the feedback issue body never disagree on the + fallback string.""" + from openkb import __version__ + from openkb.cli import _openkb_version + + assert _openkb_version() == __version__