From 9ca027042906569196539043eedb11957955f95f Mon Sep 17 00:00:00 2001 From: mountain Date: Sun, 17 May 2026 11:15:09 +0800 Subject: [PATCH 1/3] feat(cli): add 'openkb feedback' to file a prefilled GitHub issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Submitting feedback from a CLI tool with no backend is awkward — auto-creating issues requires a maintainer-owned token (security nightmare to ship in source), running an OpenKB-owned API server is overkill for an OSS CLI, and asking users to authenticate with their own gh CLI excludes anyone who hasn't installed it. Workaround: build a GitHub issue URL with title / body / labels prefilled in query params, and open the user's browser. The user goes through GitHub's normal flow with their own account — no backend, no secrets, no auth dance. Usage: openkb feedback # interactive (stdin) openkb feedback "openkb add hangs" # one-liner openkb feedback --type bug "..." # tags 'bug' label openkb feedback --print-url "..." # SSH / no-browser env openkb feedback --no-diagnostics "..." # skip env info The auto-collected diagnostics are deliberately minimal — openkb version, Python version, OS, whether a KB is initialised in cwd. No paths, no env vars, no API keys. Users can disable with --no-diagnostics if even that's too much. Implementation is ~90 lines in cli.py plus 17 regression tests covering URL shape, type→label mapping, title truncation, the --no-diagnostics body shape, and the webbrowser-not-called contract for --print-url. --- README.md | 1 + openkb/cli.py | 146 +++++++++++++++++++++++++++++ tests/test_feedback.py | 207 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 354 insertions(+) create mode 100644 tests/test_feedback.py diff --git a/README.md b/README.md index c1640659..c53ebdfe 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`, `--print-url` for SSH / sandbox) | diff --git a/openkb/cli.py b/openkb/cli.py index d574ef1e..cba99b92 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -1133,3 +1133,149 @@ 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, or 'unknown' if it + can't be resolved (e.g. running from an editable checkout that isn't + on PYTHONPATH as a distribution). + """ + try: + from importlib.metadata import version + return version("openkb") + except Exception: + return "unknown" + + +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.option( + "--print-url", is_flag=True, default=False, + help="Print the prefilled GitHub issue URL instead of opening a browser.", +) +@click.option( + "--no-diagnostics", is_flag=True, default=False, + help="Don't attach the diagnostics block (openkb version, Python, OS, KB present).", +) +@click.pass_context +def feedback(ctx, message, feedback_type, print_url, no_diagnostics): + """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' + openkb feedback --print-url "..." # SSH / sandbox-friendly + + 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: + feedback_type = click.prompt( + "Type", + default="other", + type=click.Choice(_FEEDBACK_TYPES), + show_default=True, + show_choices=True, + ) + + diagnostics = {} if no_diagnostics else _collect_feedback_diagnostics(ctx) + url = _build_feedback_url(message, feedback_type, diagnostics) + + if print_url: + click.echo(url) + return + + click.echo("Opening GitHub in your browser to file the issue.") + click.echo("If nothing happens, copy this URL into a browser yourself:") + click.echo(f" {url}") + + import webbrowser + try: + 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) diff --git a/tests/test_feedback.py b/tests/test_feedback.py new file mode 100644 index 00000000..c02c3e6c --- /dev/null +++ b/tests/test_feedback.py @@ -0,0 +1,207 @@ +"""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(): + """Empty dict means user passed --no-diagnostics; body is just the message.""" + 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_print_url_does_not_open_browser(): + """--print-url is the SSH / sandbox path: it must NOT call webbrowser.open.""" + runner = CliRunner() + with patch("webbrowser.open") as mock_open: + result = runner.invoke( + cli, ["feedback", "--type", "bug", "--print-url", "openkb crashes on .ppt"], + ) + + assert result.exit_code == 0, result.output + assert "https://github.com/VectifyAI/OpenKB/issues/new" in result.output + mock_open.assert_not_called() + + +def test_feedback_one_liner_default_opens_browser_with_url(): + 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 fallback URL should also be printed so the user has a copy if the + # 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_no_diagnostics_strips_block_from_body(): + runner = CliRunner() + with patch("webbrowser.open") as mock_open: + result = runner.invoke( + cli, + ["feedback", "--type", "bug", "--print-url", "--no-diagnostics", "just this"], + ) + + assert result.exit_code == 0 + url = result.output.strip().splitlines()[-1] + assert "Diagnostics" not in url + assert "details" not in url # no
block + mock_open.assert_not_called() + + +def test_feedback_prompts_for_type_when_not_given_via_flag(): + """If --type isn't on the command line, prompt the user.""" + runner = CliRunner() + with patch("webbrowser.open") as mock_open: + result = runner.invoke( + cli, ["feedback", "--print-url", "missing-type prompt test"], + input="feature\n", + ) + + assert result.exit_code == 0 + # The URL should be tagged with the chosen type (mapped to 'enhancement'). + url_line = [ln for ln in result.output.splitlines() if "issues/new" in ln][-1] + assert "labels=enhancement" in url_line From 18dd708f8b1395e096c6d6a45f73e417a7208570 Mon Sep 17 00:00:00 2001 From: mountain Date: Sun, 17 May 2026 11:57:55 +0800 Subject: [PATCH 2/3] fix(feedback): address 3 self-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. **webbrowser.open return value silently dropped on headless boxes** The command printed "Opening GitHub in your browser..." and exited 0 even when webbrowser.open returned False (no GUI, no $BROWSER, CI runner, container without DISPLAY). Now we capture the return value, surface "no browser available — copy the URL above" to stderr, and only print "Opened GitHub in your browser." after a confirmed True return. 2. **_openkb_version was the third copy of the same logic** openkb/__init__.py exports __version__ via importlib.metadata with fallback "0.0.0+unknown"; openkb/agent/chat.py wraps it as _openkb_version(); cli.py was re-implementing it with fallback "unknown" — already drifted. Replaced with the same one-liner used in chat.py: `from openkb import __version__; return __version__`. Now all three call sites agree. 3. **type prompt hung in non-TTY contexts (regression of PR #48)** PR #48 introduced _stdin_is_tty() specifically because adding a prompt without a TTY check broke automation pipelines. PR #53's second prompt (asking for feedback type) repeated the same pattern. Now it skips the prompt when stdin isn't a TTY and falls through to `--type other`. Pipes / CI work without flags now: echo "..." | openkb feedback "msg" # works, type=other, no label 4 new regression tests: - test_feedback_skips_type_prompt_when_stdin_is_not_a_tty - test_feedback_warns_when_webbrowser_open_returns_false - test_feedback_confirms_when_webbrowser_open_succeeds - test_openkb_version_helper_matches_package_version 331 tests pass (327 prior + 4 new). --- openkb/cli.py | 54 +++++++++++++++++++++++------------ tests/test_feedback.py | 65 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 19 deletions(-) diff --git a/openkb/cli.py b/openkb/cli.py index cba99b92..6033996b 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -1150,15 +1150,15 @@ def status(ctx): def _openkb_version() -> str: - """Return the installed openkb package version, or 'unknown' if it - can't be resolved (e.g. running from an editable checkout that isn't - on PYTHONPATH as a distribution). + """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``. """ - try: - from importlib.metadata import version - return version("openkb") - except Exception: - return "unknown" + from openkb import __version__ + return __version__ def _collect_feedback_diagnostics(ctx) -> dict[str, str]: @@ -1253,13 +1253,20 @@ def feedback(ctx, message, feedback_type, print_url, no_diagnostics): return if feedback_type is None: - feedback_type = click.prompt( - "Type", - default="other", - type=click.Choice(_FEEDBACK_TYPES), - show_default=True, - show_choices=True, - ) + # 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 = {} if no_diagnostics else _collect_feedback_diagnostics(ctx) url = _build_feedback_url(message, feedback_type, diagnostics) @@ -1268,14 +1275,25 @@ def feedback(ctx, message, feedback_type, print_url, no_diagnostics): click.echo(url) return - click.echo("Opening GitHub in your browser to file the issue.") - click.echo("If nothing happens, copy this URL into a browser yourself:") + click.echo("Copy this URL into a browser if the auto-open below fails:") click.echo(f" {url}") import webbrowser try: - webbrowser.open(url) + 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 index c02c3e6c..fa07b5c9 100644 --- a/tests/test_feedback.py +++ b/tests/test_feedback.py @@ -195,7 +195,8 @@ def test_feedback_no_diagnostics_strips_block_from_body(): def test_feedback_prompts_for_type_when_not_given_via_flag(): """If --type isn't on the command line, prompt the user.""" runner = CliRunner() - with patch("webbrowser.open") as mock_open: + with patch("webbrowser.open") as mock_open, \ + patch("openkb.cli._stdin_is_tty", return_value=True): result = runner.invoke( cli, ["feedback", "--print-url", "missing-type prompt test"], input="feature\n", @@ -205,3 +206,65 @@ def test_feedback_prompts_for_type_when_not_given_via_flag(): # The URL should be tagged with the chosen type (mapped to 'enhancement'). 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", "--print-url", "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__ From dad19e5e42aa97d5dcae61dd3b853c71f9bd64eb Mon Sep 17 00:00:00 2001 From: mountain Date: Sun, 17 May 2026 12:19:06 +0800 Subject: [PATCH 3/3] docs(feedback): drop --print-url and --no-diagnostics, keep one flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two flags added complexity without changing the day-one user experience: - --print-url: URL is already printed before webbrowser.open is attempted (the "Copy this URL..." line), so SSH/sandbox users can copy it from the terminal regardless of whether the auto-open succeeds. The flag was redundant. - --no-diagnostics: the diagnostics block is intentionally tiny (openkb version, Python, OS, kb_initialised yes/no) — no paths, usernames, env vars, or keys. Maintainers benefit from having it on every issue without exception, and the privacy cost is negligible. Surface shrinks to one flag (--type) plus the positional message. Help text and README updated. 19 feedback tests still pass (329 total). --- README.md | 2 +- openkb/cli.py | 17 ++------------ tests/test_feedback.py | 50 ++++++++++-------------------------------- 3 files changed, 14 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index c53ebdfe..4dd51c47 100644 --- a/README.md +++ b/README.md @@ -156,7 +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`, `--print-url` for SSH / sandbox) | +| 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 6033996b..5197a930 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -1216,16 +1216,8 @@ def _build_feedback_url( default=None, help="Feedback type — sets the GitHub issue label.", ) -@click.option( - "--print-url", is_flag=True, default=False, - help="Print the prefilled GitHub issue URL instead of opening a browser.", -) -@click.option( - "--no-diagnostics", is_flag=True, default=False, - help="Don't attach the diagnostics block (openkb version, Python, OS, KB present).", -) @click.pass_context -def feedback(ctx, message, feedback_type, print_url, no_diagnostics): +def feedback(ctx, message, feedback_type): """Submit feedback by opening a prefilled GitHub issue. Examples: @@ -1234,7 +1226,6 @@ def feedback(ctx, message, feedback_type, print_url, no_diagnostics): openkb feedback # interactive openkb feedback "openkb add hangs on .docx" # one-line bug report openkb feedback --type feature "..." # tags the issue 'enhancement' - openkb feedback --print-url "..." # SSH / sandbox-friendly The command does not send anything to OpenKB maintainers directly — it opens GitHub in your browser with title, body, and label prefilled. @@ -1268,13 +1259,9 @@ def feedback(ctx, message, feedback_type, print_url, no_diagnostics): else: feedback_type = "other" - diagnostics = {} if no_diagnostics else _collect_feedback_diagnostics(ctx) + diagnostics = _collect_feedback_diagnostics(ctx) url = _build_feedback_url(message, feedback_type, diagnostics) - if print_url: - click.echo(url) - return - click.echo("Copy this URL into a browser if the auto-open below fails:") click.echo(f" {url}") diff --git a/tests/test_feedback.py b/tests/test_feedback.py index fa07b5c9..96719de0 100644 --- a/tests/test_feedback.py +++ b/tests/test_feedback.py @@ -96,7 +96,9 @@ def test_build_url_diagnostics_attached_when_provided(): def test_build_url_no_diagnostics_block_when_empty(): - """Empty dict means user passed --no-diagnostics; body is just the message.""" + """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" @@ -141,20 +143,8 @@ class _Ctx: # --------------------------------------------------------------------------- -def test_feedback_one_liner_print_url_does_not_open_browser(): - """--print-url is the SSH / sandbox path: it must NOT call webbrowser.open.""" - runner = CliRunner() - with patch("webbrowser.open") as mock_open: - result = runner.invoke( - cli, ["feedback", "--type", "bug", "--print-url", "openkb crashes on .ppt"], - ) - - assert result.exit_code == 0, result.output - assert "https://github.com/VectifyAI/OpenKB/issues/new" in result.output - mock_open.assert_not_called() - - -def test_feedback_one_liner_default_opens_browser_with_url(): +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"]) @@ -163,8 +153,7 @@ def test_feedback_one_liner_default_opens_browser_with_url(): 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 fallback URL should also be printed so the user has a copy if the - # auto-open fails. + # The URL is also printed so the user has a copy if auto-open fails. assert called_url in result.output @@ -177,33 +166,18 @@ def test_feedback_empty_message_aborts_with_exit_1(): assert "No feedback provided" in result.output -def test_feedback_no_diagnostics_strips_block_from_body(): - runner = CliRunner() - with patch("webbrowser.open") as mock_open: - result = runner.invoke( - cli, - ["feedback", "--type", "bug", "--print-url", "--no-diagnostics", "just this"], - ) - - assert result.exit_code == 0 - url = result.output.strip().splitlines()[-1] - assert "Diagnostics" not in url - assert "details" not in url # no
block - mock_open.assert_not_called() - - def test_feedback_prompts_for_type_when_not_given_via_flag(): - """If --type isn't on the command line, prompt the user.""" + """If --type isn't on the command line and stdin is a TTY, prompt the user.""" runner = CliRunner() - with patch("webbrowser.open") as mock_open, \ + with patch("webbrowser.open"), \ patch("openkb.cli._stdin_is_tty", return_value=True): result = runner.invoke( - cli, ["feedback", "--print-url", "missing-type prompt test"], + cli, ["feedback", "missing-type prompt test"], input="feature\n", ) assert result.exit_code == 0 - # The URL should be tagged with the chosen type (mapped to 'enhancement'). + # 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 @@ -219,9 +193,7 @@ def test_feedback_skips_type_prompt_when_stdin_is_not_a_tty(): runner = CliRunner() with patch("webbrowser.open"), \ patch("openkb.cli._stdin_is_tty", return_value=False): - result = runner.invoke( - cli, ["feedback", "--print-url", "non-tty feedback"], - ) + 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