diff --git a/.github/dependabot.yml b/.github/dependabot.yml index be4fe4b5..9a29f805 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -8,4 +8,23 @@ updates: - package-ecosystem: "uv" directory: "/" schedule: - interval: "daily" + interval: "weekly" + day: "monday" + # Cap concurrent bot PRs so the queue stays reviewable. + open-pull-requests-limit: 3 + # Consolidate routine bumps into a single PR; isolate majors so + # breaking changes get their own review. + groups: + minor-and-patch: + update-types: + - "minor" + - "patch" + ignore: + # ruff is deliberately pinned `<0.15` in pyproject.toml; 0.15's formatter + # reflow fails `make check`. Don't let Dependabot widen the ceiling. + - dependency-name: "ruff" + versions: [">=0.15"] + # click 8.4.x regresses pyright (`click.Option` typed partially unknown, + # ~95 errors in `make check`). Hold until the type regression is resolved. + - dependency-name: "click" + versions: [">=8.4"] diff --git a/CHANGELOG.md b/CHANGELOG.md index bda9e5b1..7e4e11b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **`/feedback` submits structured reports with redacted session context.** A new + `/feedback [bug|feature|ux|wrong] [message]` command collects recent session context + and strips sensitive file contents before sending, shows a confirmation preview, and + falls back to opening a prefilled GitHub issue when direct submission is unavailable. - **Fresh releases surface in the startup update prompt immediately.** The pre-start update prompt now revalidates a cached "already current" answer with a bounded conditional request instead of waiting for the 24-hour background-check throttle. diff --git a/examples/feedback-worker/src/index.ts b/examples/feedback-worker/src/index.ts index 7176ecdb..889f575a 100644 --- a/examples/feedback-worker/src/index.ts +++ b/examples/feedback-worker/src/index.ts @@ -20,6 +20,7 @@ type RecentError = { }; type FeedbackPayload = { + schema_version?: number; session_id?: string; type?: string; content?: string; @@ -27,10 +28,26 @@ type FeedbackPayload = { os?: string; model?: string; recent_errors?: RecentError[]; + session?: Record; + client?: Record; + repo?: Record; + context?: { + recent_errors?: RecentError[]; + last_messages?: unknown[]; + tool_calls?: unknown[]; + subagents?: unknown[]; + }; + privacy?: Record; +}; + +type GitHubIssue = { + number?: number; + html_url?: string; }; const MAX_CONTENT_LENGTH = 10_000; const MAX_RECENT_ERRORS = 10; +const MAX_CONTEXT_ITEMS = 20; export default { async fetch(request: Request, env: Env): Promise { @@ -50,15 +67,17 @@ export default { let payload: FeedbackPayload; try { - payload = await request.json(); + payload = (await request.json()) as FeedbackPayload; } catch { return jsonResponse({ error: "invalid_json" }, 400); } const content = (payload.content || "").trim(); - const recentErrors = Array.isArray(payload.recent_errors) - ? payload.recent_errors.slice(0, MAX_RECENT_ERRORS) - : []; + const recentErrors = Array.isArray(payload.context?.recent_errors) + ? payload.context.recent_errors.slice(0, MAX_RECENT_ERRORS) + : Array.isArray(payload.recent_errors) + ? payload.recent_errors.slice(0, MAX_RECENT_ERRORS) + : []; if (!content && recentErrors.length === 0) { return jsonResponse({ error: "empty_feedback" }, 400); } @@ -74,15 +93,25 @@ export default { os: trim(payload.os, 128), model: trim(payload.model, 128), recent_errors: recentErrors.map(sanitizeRecentError), + session: sanitizeRecord(payload.session), + client: sanitizeRecord(payload.client), + repo: sanitizeRecord(payload.repo, 24, 20_000), + context: { + recent_errors: recentErrors.map(sanitizeRecentError), + last_messages: sanitizeArray(payload.context?.last_messages), + tool_calls: sanitizeArray(payload.context?.tool_calls), + subagents: sanitizeArray(payload.context?.subagents), + }, + privacy: sanitizeRecord(payload.privacy), }; const title = githubTitle(sanitizedPayload); const body = githubBody(sanitizedPayload, request); - await createGithubIssue(env, title, body); + const issue = await createGithubIssue(env, sanitizedPayload, title, body); await sendSupportEmail(env, title, body); - return corsResponse(null, 204); + return jsonResponse({ number: issue.number, html_url: issue.html_url }, 201); }, }; @@ -103,6 +132,33 @@ function sanitizeRecentError(error: RecentError): RecentError { }; } +function sanitizeValue(value: unknown, maxStringLength = 2_000): unknown { + if (typeof value === "string") return value.slice(0, maxStringLength); + if (typeof value === "number" || typeof value === "boolean" || value === null) return value; + if (Array.isArray(value)) return value.slice(0, MAX_CONTEXT_ITEMS).map((item) => sanitizeValue(item)); + if (typeof value === "object" && value !== null) return sanitizeRecord(value as Record); + return undefined; +} + +function sanitizeRecord( + value: unknown, + maxKeys = 20, + maxStringLength = 2_000, +): Record | undefined { + if (typeof value !== "object" || value === null || Array.isArray(value)) return undefined; + const out: Record = {}; + for (const [key, raw] of Object.entries(value).slice(0, maxKeys)) { + out[key.slice(0, 80)] = sanitizeValue(raw, maxStringLength); + } + return out; +} + +function sanitizeArray(value: unknown): unknown[] | undefined { + return Array.isArray(value) + ? value.slice(0, MAX_CONTEXT_ITEMS).map((item) => sanitizeValue(item)) + : undefined; +} + function githubTitle(payload: FeedbackPayload): string { const prefix = payload.type === "error" ? "Error report" : "Feedback"; const version = payload.version ? ` ${payload.version}` : ""; @@ -127,6 +183,9 @@ function githubBody(payload: FeedbackPayload, request: Request): string { `- CF ray: ${request.headers.get("cf-ray") || "unknown"}`, ]; + appendJsonSection(lines, "Privacy", payload.privacy); + appendJsonSection(lines, "Repository", payload.repo); + if (payload.recent_errors?.length) { lines.push("", "## Recent errors", ""); for (const error of payload.recent_errors) { @@ -138,11 +197,30 @@ function githubBody(payload: FeedbackPayload, request: Request): string { } } + appendJsonSection(lines, "Recent visible messages", payload.context?.last_messages); + appendJsonSection(lines, "Tool calls", payload.context?.tool_calls); + appendJsonSection(lines, "Subagents", payload.context?.subagents); + return lines.join("\n"); } -async function createGithubIssue(env: Env, title: string, body: string): Promise { - const labels = splitCsv(env.GITHUB_LABELS || "feedback,pythinker-cli"); +function appendJsonSection(lines: string[], title: string, value: unknown): void { + if (value === undefined || value === null) return; + if (Array.isArray(value) && value.length === 0) return; + if (typeof value === "object" && !Array.isArray(value) && Object.keys(value).length === 0) return; + lines.push("", `## ${title}`, "", "```json", JSON.stringify(value, null, 2), "```"); +} + +async function createGithubIssue( + env: Env, + payload: FeedbackPayload, + title: string, + body: string, +): Promise { + const labels = unique([ + ...splitCsv(env.GITHUB_LABELS || "feedback,pythinker-cli"), + `feedback:${payload.type || "feedback"}`, + ]); const assignees = splitCsv(env.GITHUB_ASSIGNEES || ""); const response = await fetch(`https://api.github.com/repos/${env.GITHUB_REPO}/issues`, { method: "POST", @@ -159,6 +237,8 @@ async function createGithubIssue(env: Env, title: string, body: string): Promise if (!response.ok) { throw new Error(`GitHub issue creation failed: ${response.status}`); } + const issue = (await response.json()) as GitHubIssue; + return { number: issue.number, html_url: issue.html_url }; } async function sendSupportEmail(env: Env, subject: string, body: string): Promise { @@ -229,6 +309,10 @@ function splitCsv(value: string): string[] { .filter(Boolean); } +function unique(values: string[]): string[] { + return Array.from(new Set(values)); +} + function corsResponse(body: BodyInit | null, status: number): Response { return new Response(body, { status, headers: corsHeaders() }); } diff --git a/src/pythinker_code/feedback.py b/src/pythinker_code/feedback.py new file mode 100644 index 00000000..6f45df75 --- /dev/null +++ b/src/pythinker_code/feedback.py @@ -0,0 +1,587 @@ +from __future__ import annotations + +import asyncio +import json +import platform +import re +import shlex +import subprocess +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path +from typing import TYPE_CHECKING, Any, Literal, cast + +import aiohttp + +from pythinker_code.constant import VERSION +from pythinker_code.telemetry.errors import RecentError, recent_errors +from pythinker_code.ui.shell.oauth import current_model_key +from pythinker_code.utils.aiohttp import new_client_session +from pythinker_code.utils.export import is_sensitive_file +from pythinker_code.utils.logging import logger +from pythinker_code.utils.string import shorten +from pythinker_code.wire.types import TextPart, ThinkPart + +if TYPE_CHECKING: + from pythinker_core.message import Message + + from pythinker_code.soul.pythinkersoul import PythinkerSoul + +FeedbackType = Literal["bug", "feature", "ux", "wrong", "other"] + +FEEDBACK_TYPES: set[str] = {"bug", "feature", "ux", "wrong", "other"} +_TYPE_ALIASES: dict[str, FeedbackType] = { + "bug": "bug", + "error": "bug", + "crash": "bug", + "feature": "feature", + "request": "feature", + "ux": "ux", + "ui": "ux", + "wrong": "wrong", + "incorrect": "wrong", + "bad": "wrong", + "other": "other", + "feedback": "other", +} +_SENSITIVE_KEY_RE = re.compile(r"(api[_-]?key|token|secret|password|passwd|authorization)", re.I) +_SECRET_PATTERNS: tuple[tuple[re.Pattern[str], str], ...] = ( + ( + re.compile(r"(?i)(authorization\s*[:=]\s*(?:bearer|token)\s+)[A-Za-z0-9._~+/=-]{8,}"), + r"\1", + ), + ( + re.compile(r"(?i)((?:api[_-]?key|token|secret|password|passwd)\s*[:=]\s*)[^\s'\"]+"), + r"\1", + ), + (re.compile(r"\bsk-(?:ant|proj|[A-Za-z0-9])[A-Za-z0-9_-]{16,}\b"), ""), + (re.compile(r"\bxox(?:a|b|p|r|s)-[A-Za-z0-9-]{10,}\b"), ""), + (re.compile(r"\bAIza[0-9A-Za-z_-]{30,45}\b"), ""), + ( + re.compile(r"\beyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b"), + "", + ), + (re.compile(r"gh[pousr]_[A-Za-z0-9_]{20,}"), ""), + (re.compile(r"AKIA[0-9A-Z]{16}"), ""), + ( + re.compile( + r"-----BEGIN [A-Z ]*PRIVATE KEY-----.*?-----END [A-Z ]*PRIVATE KEY-----", + re.S, + ), + "", + ), +) +_HINT_KEYS = ("path", "file_path", "command", "query", "url", "name", "pattern") +_MAX_MESSAGE_TEXT = 1_500 +_MAX_MESSAGES_DEFAULT = 10 +_MAX_MESSAGES_WITH_TRANSCRIPT = 80 +_MAX_TOOL_CALLS = 40 +_MAX_DIFF_CHARS = 60_000 +_MAX_COMMAND_OUTPUT = 20_000 +_MAX_ISSUE_URL_BODY_CHARS = 5_500 +_MAX_ISSUE_URL_CONTENT_CHARS = 2_500 + + +@dataclass(slots=True, frozen=True) +class FeedbackOptions: + kind: FeedbackType + message: str + include_diff: bool = False + include_transcript: bool = False + include_tool_details: bool = False + yes: bool = False + + @property + def includes_sensitive_context(self) -> bool: + return self.include_diff or self.include_transcript or self.include_tool_details + + +@dataclass(slots=True, frozen=True) +class FeedbackSubmission: + number: int | None = None + html_url: str | None = None + + +def parse_feedback_args(args: str) -> FeedbackOptions | str: + """Parse `/feedback` args into a structured request or an error string.""" + try: + parts = shlex.split(args) + except ValueError as exc: + return f"Invalid /feedback arguments: {exc}" + + kind: FeedbackType = "other" + include_diff = False + include_transcript = False + include_tool_details = False + yes = False + message_parts: list[str] = [] + + for part in parts: + if part in {"--include-diff", "--diff"}: + include_diff = True + elif part in {"--include-transcript", "--transcript"}: + include_transcript = True + elif part in {"--include-tool-details", "--tool-details"}: + include_tool_details = True + elif part in {"--yes", "-y"}: + yes = True + elif part.startswith("--"): + return f"Unknown /feedback option: {part}" + elif not message_parts and kind == "other" and part.lower() in _TYPE_ALIASES: + kind = _TYPE_ALIASES[part.lower()] + else: + message_parts.append(part) + + return FeedbackOptions( + kind=kind, + message=" ".join(message_parts).strip(), + include_diff=include_diff, + include_transcript=include_transcript, + include_tool_details=include_tool_details, + yes=yes, + ) + + +def redact_text(text: str) -> str: + redacted = text + for pattern, replacement in _SECRET_PATTERNS: + redacted = pattern.sub(replacement, redacted) + try: + home = str(Path.home()) + if home and home != "/": + redacted = redacted.replace(home, "~") + except (RuntimeError, OSError): + logger.debug("Could not resolve home directory for redaction") + return redacted + + +def redact_value(value: object, *, key: str = "") -> object: + if _SENSITIVE_KEY_RE.search(key): + return "" + if isinstance(value, str): + return redact_text(value) + if isinstance(value, Mapping): + values = cast(Mapping[object, object], value) + return {str(k): redact_value(v, key=str(k)) for k, v in values.items()} + if isinstance(value, list | tuple): + values = cast(list[object] | tuple[object, ...], value) + return [redact_value(v) for v in values] + return value + + +def feedback_summary(payload: dict[str, Any]) -> str: + privacy = cast(dict[str, Any], payload.get("privacy") or {}) + context = cast(dict[str, Any], payload.get("context") or {}) + repo = cast(dict[str, Any], payload.get("repo") or {}) + lines = ["Pythinker will include:"] + lines.append("✓ session id, version, OS, Python, active model") + if repo: + lines.append("✓ git branch/head and diffstat") + if context.get("recent_errors"): + lines.append(f"✓ {len(context['recent_errors'])} recent error(s)") + if context.get("last_messages"): + lines.append(f"✓ {len(context['last_messages'])} recent visible message(s)") + if context.get("tool_calls"): + lines.append(f"✓ {len(context['tool_calls'])} tool call summary item(s)") + lines.append("✓ best-effort secret/path redaction") + lines.append("✓ patch diff" if privacy.get("included_diff") else "✗ patch diff") + lines.append( + "✓ extended transcript" if privacy.get("included_transcript") else "✗ extended transcript" + ) + lines.append( + "✓ tool args/results" + if privacy.get("included_tool_details") + else "✗ detailed tool args/results" + ) + return "\n".join(lines) + + +async def build_feedback_payload(soul: PythinkerSoul, options: FeedbackOptions) -> dict[str, Any]: + session = soul.runtime.session + errors = recent_errors() + repo = await _collect_git_snapshot(Path(str(session.work_dir)), options) + context = _collect_context_snapshot(soul, options, errors) + payload: dict[str, Any] = { + "schema_version": 1, + "type": options.kind, + "content": redact_text(options.message), + # Compatibility with the existing feedback worker. + "session_id": session.id, + "version": VERSION, + "os": f"{platform.system()} {platform.release()}", + "model": current_model_key(soul), + "session": { + "id": session.id, + "title": getattr(session, "title", "") or "", + "role": soul.runtime.role, + }, + "client": { + "version": VERSION, + "os": f"{platform.system()} {platform.release()}", + "python": platform.python_version(), + "model": current_model_key(soul), + "agent": soul.name, + }, + "repo": repo, + "context": context, + "privacy": { + "redacted": True, + "included_diff": options.include_diff, + "included_transcript": options.include_transcript, + "included_tool_details": options.include_tool_details, + }, + } + return cast(dict[str, Any], redact_value(payload)) + + +async def submit_feedback_payload( + feedback_url: str, + headers: dict[str, str], + payload: dict[str, Any], +) -> FeedbackSubmission: + async with ( + new_client_session() as session, + session.post( + feedback_url, json=payload, headers=headers, raise_for_status=True + ) as response, + ): + if response.status == 204: + return FeedbackSubmission() + try: + data_any: Any = await response.json(content_type=None) + except (aiohttp.ContentTypeError, ValueError): + return FeedbackSubmission() + if not isinstance(data_any, dict): + return FeedbackSubmission() + data = cast(dict[str, object], data_any) + number = data.get("number") + html_url = data.get("html_url") + return FeedbackSubmission( + number=number if isinstance(number, int) else None, + html_url=html_url if isinstance(html_url, str) and html_url else None, + ) + + +def build_feedback_issue_url( + payload: dict[str, Any], repo: str = "TechMatrix-labs/pythinker-code" +) -> str: + from urllib.parse import urlencode + + title = build_feedback_title(payload) + body = build_feedback_issue_body(payload) + labels = f"feedback,feedback:{payload.get('type') or 'other'}" + return f"https://github.com/{repo}/issues/new?" + urlencode( + {"title": title, "body": body, "labels": labels} + ) + + +def build_feedback_title(payload: dict[str, Any]) -> str: + kind = str(payload.get("type") or "other") + message = str(payload.get("content") or "").strip().splitlines()[0:1] + suffix = f": {shorten(message[0], width=70)}" if message else "" + return f"[Pythinker CLI] {kind.title()} feedback{suffix}" + + +def build_feedback_issue_body(payload: dict[str, Any]) -> str: + """Compact GitHub URL fallback body. + + GitHub's ``issues/new?body=...`` path has practical URL-length limits, so + this intentionally omits rich transcript/tool/diff context. The structured + worker/API path receives the full JSON payload instead. + """ + session = cast(dict[str, Any], payload.get("session") or {}) + client = cast(dict[str, Any], payload.get("client") or {}) + repo = cast(dict[str, Any], payload.get("repo") or {}) + context = cast(dict[str, Any], payload.get("context") or {}) + privacy = cast(dict[str, Any], payload.get("privacy") or {}) + content = str(payload.get("content") or "_(no comment)_") + lines = [ + "## User submission", + "", + _truncate(content, _MAX_ISSUE_URL_CONTENT_CHARS), + "", + "## Compact fallback context", + "", + f"- Type: {payload.get('type') or 'other'}", + f"- Session: {session.get('id') or payload.get('session_id') or 'unknown'}", + f"- Version: {client.get('version') or payload.get('version') or 'unknown'}", + f"- OS: {client.get('os') or payload.get('os') or 'unknown'}", + f"- Python: {client.get('python') or 'unknown'}", + f"- Model: {client.get('model') or payload.get('model') or 'unknown'}", + f"- Redacted: {privacy.get('redacted', True)}", + ] + if repo: + lines.extend( + [ + f"- Branch: {repo.get('branch') or 'unknown'}", + f"- HEAD: {repo.get('head') or 'unknown'}", + f"- Dirty: {repo.get('dirty', 'unknown')}", + ] + ) + if context.get("recent_errors"): + lines.append(f"- Recent errors: {len(context['recent_errors'])}") + if context.get("last_messages"): + lines.append(f"- Recent visible messages omitted: {len(context['last_messages'])}") + if context.get("tool_calls"): + lines.append(f"- Tool call summaries omitted: {len(context['tool_calls'])}") + if repo.get("diff"): + lines.append("- Patch diff omitted: GitHub fallback URLs are length-limited.") + lines.extend( + [ + "", + "> Full structured context was omitted from this browser fallback because " + "GitHub issue URLs are length-limited.", + ] + ) + return _truncate("\n".join(lines), _MAX_ISSUE_URL_BODY_CHARS) + + +def build_feedback_body(payload: dict[str, Any]) -> str: + session = cast(dict[str, Any], payload.get("session") or {}) + client = cast(dict[str, Any], payload.get("client") or {}) + repo = cast(dict[str, Any], payload.get("repo") or {}) + context = cast(dict[str, Any], payload.get("context") or {}) + privacy = cast(dict[str, Any], payload.get("privacy") or {}) + lines = [ + "## User submission", + "", + str(payload.get("content") or "_(no comment)_"), + "", + "## Context", + "", + f"- Type: {payload.get('type') or 'other'}", + f"- Session: {session.get('id') or payload.get('session_id') or 'unknown'}", + f"- Version: {client.get('version') or payload.get('version') or 'unknown'}", + f"- OS: {client.get('os') or payload.get('os') or 'unknown'}", + f"- Python: {client.get('python') or 'unknown'}", + f"- Model: {client.get('model') or payload.get('model') or 'unknown'}", + f"- Redacted: {privacy.get('redacted', True)}", + ] + if repo: + lines.extend( + [ + "", + "## Repository", + "", + f"- Branch: {repo.get('branch') or 'unknown'}", + f"- HEAD: {repo.get('head') or 'unknown'}", + f"- Dirty: {repo.get('dirty', 'unknown')}", + ] + ) + if repo.get("diff_stat"): + lines.extend(["", "```text", str(repo["diff_stat"]), "```"]) + if repo.get("diff"): + lines.extend( + [ + "", + "
Patch diff", + "", + "```diff", + str(repo["diff"]), + "```", + "
", + ] + ) + + if context.get("recent_errors"): + lines.extend(["", "## Recent errors", ""]) + for error in cast(list[dict[str, Any]], context["recent_errors"]): + lines.append( + f"- {error.get('site') or 'unknown'}: {error.get('exc_class') or 'unknown'}" + f"{f' (tool={error.get("tool")})' if error.get('tool') else ''}" + f"{f' — {error.get("message")}' if error.get('message') else ''}" + ) + + if context.get("last_messages"): + lines.extend(["", "## Recent visible messages", ""]) + for message in cast(list[dict[str, Any]], context["last_messages"]): + lines.append(f"### {message.get('role', 'message')}") + lines.append("") + lines.append(str(message.get("text") or "")) + lines.append("") + + if context.get("tool_calls"): + lines.extend(["", "## Tool calls", ""]) + for call in cast(list[dict[str, Any]], context["tool_calls"]): + hint = f" — {call.get('hint')}" if call.get("hint") else "" + lines.append(f"- {call.get('name') or 'unknown'}{hint}") + + return "\n".join(lines) + + +async def _collect_git_snapshot(work_dir: Path, options: FeedbackOptions) -> dict[str, Any]: + async def git(*args: str) -> str: + return await asyncio.to_thread(_run_git, work_dir, list(args)) + + branch = await git("rev-parse", "--abbrev-ref", "HEAD") + if not branch: + return {} + head, status, unstaged_stat, cached_stat = await asyncio.gather( + git("rev-parse", "--short", "HEAD"), + git("status", "--short"), + git("diff", "--stat"), + git("diff", "--cached", "--stat"), + ) + diff_stat = "\n".join(part for part in [unstaged_stat, cached_stat] if part.strip()) + snapshot: dict[str, Any] = { + "branch": branch, + "head": head, + "dirty": bool(status.strip()), + "status_short": status[:_MAX_COMMAND_OUTPUT], + "diff_stat": diff_stat[:_MAX_COMMAND_OUTPUT], + } + if options.include_diff: + unstaged_diff, cached_diff = await asyncio.gather( + git("diff", "--no-ext-diff"), + git("diff", "--cached", "--no-ext-diff"), + ) + diff = "\n".join(part for part in [unstaged_diff, cached_diff] if part.strip()) + snapshot["diff"] = _truncate(redact_text(diff), _MAX_DIFF_CHARS) + return snapshot + + +def _run_git(work_dir: Path, args: list[str]) -> str: + try: + proc = subprocess.run( + ["git", *args], + cwd=work_dir, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=5, + check=False, + ) + except (OSError, subprocess.TimeoutExpired): + return "" + if proc.returncode != 0: + return "" + return redact_text(proc.stdout.strip()) + + +def _collect_context_snapshot( + soul: PythinkerSoul, + options: FeedbackOptions, + errors: list[RecentError], +) -> dict[str, Any]: + history = list(soul.context.history) + message_limit = ( + _MAX_MESSAGES_WITH_TRANSCRIPT if options.include_transcript else _MAX_MESSAGES_DEFAULT + ) + return { + "recent_errors": [ + { + "timestamp": err.timestamp, + "site": err.site, + "exc_class": err.exc_class, + "message": err.message, + "tool": err.tool, + } + for err in errors + ], + "last_messages": _collect_recent_messages(history, limit=message_limit), + "tool_calls": _collect_tool_calls(history, include_details=options.include_tool_details), + "subagents": _collect_subagents(soul), + } + + +def _collect_recent_messages(history: list[Message], *, limit: int) -> list[dict[str, Any]]: + messages: list[dict[str, Any]] = [] + for message in history: + if message.role not in {"user", "assistant", "tool"}: + continue + text = _message_visible_text(message) + if not text: + continue + messages.append( + { + "role": message.role, + "text": _truncate(redact_text(text), _MAX_MESSAGE_TEXT), + } + ) + return messages[-limit:] + + +def _collect_tool_calls(history: list[Message], *, include_details: bool) -> list[dict[str, Any]]: + calls: list[dict[str, Any]] = [] + for message in history: + for tool_call in message.tool_calls or []: + args_raw = tool_call.function.arguments or "{}" + record: dict[str, Any] = { + "id": tool_call.id, + "name": tool_call.function.name, + "hint": redact_text(_extract_tool_hint(args_raw)), + } + if include_details: + record["arguments"] = redact_value(_parse_json_or_text(args_raw)) + calls.append(record) + return calls[-_MAX_TOOL_CALLS:] + + +def _collect_subagents(soul: PythinkerSoul) -> list[dict[str, Any]]: + root = soul.runtime.session.subagents_dir + records: list[dict[str, Any]] = [] + try: + meta_paths = sorted(root.glob("*/meta.json"), key=lambda p: p.stat().st_mtime, reverse=True) + except OSError: + return records + for path in meta_paths[:10]: + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + continue + if not isinstance(data, dict): + continue + data_dict = cast(dict[str, object], data) + launch_any = data_dict.get("launch_spec") + launch = cast(dict[str, object], launch_any) if isinstance(launch_any, dict) else {} + records.append( + { + "agent_id": data_dict.get("agent_id"), + "type": data_dict.get("subagent_type") or launch.get("subagent_type"), + "status": data_dict.get("status"), + "description": data_dict.get("description"), + } + ) + return records + + +def _message_visible_text(message: Message) -> str: + parts: list[str] = [] + for part in message.content: + if isinstance(part, ThinkPart): + continue + if isinstance(part, TextPart): + parts.append(part.text) + else: + part_type = getattr(part, "type", type(part).__name__) + parts.append(f"[{part_type}]") + return "\n".join(p for p in parts if p.strip()).strip() + + +def _extract_tool_hint(args_raw: str) -> str: + parsed = _parse_json_or_text(args_raw) + if not isinstance(parsed, dict): + return "" + parsed_dict = cast(dict[str, object], parsed) + for key in _HINT_KEYS: + value = parsed_dict.get(key) + if isinstance(value, str) and value.strip() and not is_sensitive_file(value): + return shorten(value, width=80) + for value in parsed_dict.values(): + if isinstance(value, str) and 0 < len(value) <= 100 and not is_sensitive_file(value): + return shorten(value, width=80) + return "" + + +def _parse_json_or_text(value: str) -> object: + try: + return cast(object, json.loads(value, strict=False)) + except (json.JSONDecodeError, TypeError): + return value + + +def _truncate(value: str, limit: int) -> str: + if len(value) <= limit: + return value + return value[:limit] + "\n… " diff --git a/src/pythinker_code/ui/shell/__init__.py b/src/pythinker_code/ui/shell/__init__.py index d8634bfb..eb1daa0d 100644 --- a/src/pythinker_code/ui/shell/__init__.py +++ b/src/pythinker_code/ui/shell/__init__.py @@ -54,9 +54,11 @@ from pythinker_code.ui.shell.slash import SKILL_COMMAND_PREFIX, shell_mode_registry from pythinker_code.ui.shell.slash import registry as shell_slash_registry from pythinker_code.ui.shell.update import ( + consume_whats_new, pending_update_notice, prompt_pre_start_update, refresh_update_cache_if_due, + welcome_update_target, ) from pythinker_code.ui.shell.visualize import ( ApprovalPromptDelegate, @@ -633,7 +635,11 @@ async def run(self, command: str | None = None) -> bool: else: self._start_background_task(self._auto_update()) - _print_welcome_info(self.soul.name or "Pythinker CLI", self._welcome_info) + _print_welcome_info( + self.soul.name or "Pythinker CLI", + self._welcome_info, + banner=_welcome_banner_chip(), + ) # Start telemetry periodic flush and disk retry from pythinker_code.telemetry import get_sink @@ -1899,7 +1905,33 @@ def _value_style_for_label(label: str, level: WelcomeInfoItem.Level) -> str: return level.value -def _print_welcome_info(name: str, info_items: list[WelcomeInfoItem]) -> None: +def _welcome_banner_chip() -> Text | None: + """One-line chip for the top-right of the welcome banner, or None. + + Precedence: update-available > what's-new > nothing. + ``consume_whats_new`` is always called first so the 'last seen' mark is + written regardless of which chip wins the display. + """ + _t = _get_tui_tokens() + whats_new_version = consume_whats_new() + update_target = welcome_update_target() + + if update_target: + chip = Text.from_markup(f"[{_t.warning}]↑ Update available — v{update_target} · /update[/]") + chip.highlight_regex(r"/[A-Za-z][A-Za-z0-9_-]*", f"bold {_t.warning}") + return chip + + if whats_new_version: + chip = Text.from_markup(f"[{_t.info}]✦ What's new in v{whats_new_version} · /changelog[/]") + chip.highlight_regex(r"/[A-Za-z][A-Za-z0-9_-]*", f"bold {_t.info}") + return chip + + return None + + +def _print_welcome_info( + name: str, info_items: list[WelcomeInfoItem], *, banner: Text | None = None +) -> None: _t = _get_tui_tokens() head = Text.from_markup("Welcome to Pythinker — think first, then code.") help_text = Text.from_markup( @@ -1911,8 +1943,16 @@ def _print_welcome_info(name: str, info_items: list[WelcomeInfoItem]) -> None: logo = Text.from_markup(_LOGO) table = Table(show_header=False, show_edge=False, box=None, padding=(0, 1), expand=False) table.add_column(justify="left") - table.add_column(justify="left", vertical="bottom") - table.add_row(logo, Group(head, help_text)) + if banner is not None: + # Chip at the top, head/help at the bottom, blank padding in between. + logo_lines = _LOGO.count("\n") + 1 # 5 for the current robot logo + pad = max(0, logo_lines - 3) # chip(1) + head(1) + help(1) = 3 fixed + right_cell: RenderableType = Group(banner, *([Text("")] * pad), head, help_text) + table.add_column(justify="left", vertical="top") + else: + right_cell = Group(head, help_text) + table.add_column(justify="left", vertical="bottom") + table.add_row(logo, right_cell) rows: list[RenderableType] = [table] diff --git a/src/pythinker_code/ui/shell/slash.py b/src/pythinker_code/ui/shell/slash.py index aab04033..dadd7082 100644 --- a/src/pythinker_code/ui/shell/slash.py +++ b/src/pythinker_code/ui/shell/slash.py @@ -578,19 +578,136 @@ def _feedback_destination(soul: PythinkerSoul) -> tuple[str, dict[str, str]] | N @registry.command @shell_mode_registry.command -def feedback(app: Shell, args: str): - """Open a GitHub issue to submit feedback or report a bug""" +async def feedback(app: Shell, args: str): + """Submit feedback with redacted session context. Usage: /feedback [bug|feature|ux|wrong] [message]""" # noqa: E501 + import aiohttp + + from pythinker_code.feedback import ( + build_feedback_issue_url, + build_feedback_payload, + feedback_summary, + parse_feedback_args, + submit_feedback_payload, + ) from pythinker_code.ui.theme import get_tui_tokens as _get_tok_fb from pythinker_code.utils.term import open_url_in_browser _t_fb = _get_tok_fb() + app_soul = getattr(app, "soul", None) + soul = app_soul if isinstance(app_soul, PythinkerSoul) else None + + def _fallback_to_issue(payload: dict[str, Any] | None = None) -> None: + issue_url = ( + build_feedback_issue_url(payload, soul.runtime.config.feedback.github_repo) + if payload is not None and soul is not None + else "https://github.com/TechMatrix-labs/pythinker-code/issues/new/choose" + ) + if open_url_in_browser(issue_url): + console.print(f"[{_t_fb.success}]Opening GitHub feedback in your browser...[/]") + else: + console.print(f"Please open: [underline]{issue_url}[/underline]") + + parsed = parse_feedback_args(args) + if isinstance(parsed, str): + console.print(f"[{_t_fb.error}]{_rich_escape(parsed)}[/]") + if soul is None: + _fallback_to_issue() + return + + if soul is None: + _fallback_to_issue() + return + + if not parsed.message: + from prompt_toolkit import PromptSession + + prompt_session: PromptSession[str] = PromptSession() + try: + message = await prompt_session.prompt_async( + f"Describe your {parsed.kind} feedback (Ctrl-C to cancel): " + ) + except (EOFError, KeyboardInterrupt): + console.print(f"[{_t_fb.muted}]Feedback cancelled.[/]") + return + parsed = type(parsed)( + kind=parsed.kind, + message=message.strip(), + include_diff=parsed.include_diff, + include_transcript=parsed.include_transcript, + include_tool_details=parsed.include_tool_details, + yes=parsed.yes, + ) + + if not parsed.message: + console.print(f"[{_t_fb.warning}]Nothing to submit. Cancelled.[/]") + return + + with console.status(f"[{_t_fb.info}]Collecting feedback context...[/]"): + payload = await build_feedback_payload(soul, parsed) + console.print(feedback_summary(payload)) - ISSUE_URL = "https://github.com/TechMatrix-labs/pythinker-code/issues/new/choose" + if not parsed.yes: + from prompt_toolkit import PromptSession - if open_url_in_browser(ISSUE_URL): - console.print(f"[{_t_fb.success}]Opening GitHub issues in your browser...[/]") + prompt_session = PromptSession[str]() + try: + answer = await prompt_session.prompt_async("Send this feedback? [y/N] ") + except (EOFError, KeyboardInterrupt): + console.print(f"[{_t_fb.muted}]Feedback cancelled.[/]") + return + if answer.strip().lower() not in {"y", "yes"}: + console.print(f"[{_t_fb.muted}]Feedback cancelled.[/]") + return + + destination = _feedback_destination(soul) + if destination is None: + _fallback_to_issue(payload) + return + feedback_url, headers = destination + + with console.status(f"[{_t_fb.info}]Submitting feedback...[/]"): + try: + submission = await submit_feedback_payload(feedback_url, headers, payload) + except TimeoutError: + console.print(f"[{_t_fb.error}]Feedback submission timed out.[/]") + _fallback_to_issue(payload) + return + except aiohttp.ClientError as exc: + status = getattr(exc, "status", None) + msg = ( + f"Feedback submission failed (HTTP {status})." + if status + else "Network error, failed to submit feedback." + ) + console.print(f"[{_t_fb.error}]{msg}[/]") + _fallback_to_issue(payload) + return + + from pythinker_code.telemetry import track + + track( + "feedback_submitted", + feedback_type=parsed.kind, + include_diff=parsed.include_diff, + include_transcript=parsed.include_transcript, + include_tool_details=parsed.include_tool_details, + ) + if submission.html_url: + console.print(f"[{_t_fb.success}]Thanks — feedback submitted: {submission.html_url}[/]") + elif submission.number is not None: + console.print( + f"[{_t_fb.success}]Thanks — feedback report #{submission.number} submitted.[/]" + ) else: - console.print(f"Please open: [underline]{ISSUE_URL}[/underline]") + console.print( + f"[{_t_fb.success}]Thanks — feedback submitted. " + f"Session ID: {soul.runtime.session.id}[/]" + ) + issue_url = build_feedback_issue_url(payload, soul.runtime.config.feedback.github_repo) + console.print( + "No report link was returned. To add follow-up details, open: " + f"[underline]{issue_url}[/underline]" + ) @registry.command(aliases=["report-error", "report"]) diff --git a/src/pythinker_code/ui/shell/update.py b/src/pythinker_code/ui/shell/update.py index a0203b99..4b249c1e 100644 --- a/src/pythinker_code/ui/shell/update.py +++ b/src/pythinker_code/ui/shell/update.py @@ -51,6 +51,7 @@ LATEST_VERSION_ETAG_FILE = get_share_dir() / "latest_version.etag" LAST_UPDATE_CHECK_FILE = get_share_dir() / "last_update_check.txt" DISMISSED_VERSION_FILE = get_share_dir() / "dismissed_update_version.txt" +LAST_SEEN_VERSION_FILE = get_share_dir() / "last_seen_version.txt" AUTO_UPDATE_CHECK_INTERVAL_SECONDS = 24 * 60 * 60 PROMPT_UPDATE_REFRESH_TIMEOUT_SECONDS = 2.0 @@ -383,6 +384,98 @@ def _skip_version_this_session(version: str) -> None: _skipped_version_this_session = version +def _read_last_seen_version() -> str | None: + try: + return LAST_SEEN_VERSION_FILE.read_text(encoding="utf-8").strip() or None + except FileNotFoundError: + return None + except OSError: + logger.exception("Failed to read last-seen version:") + return None + + +def _write_last_seen_version(version: str) -> None: + try: + LAST_SEEN_VERSION_FILE.write_text(version, encoding="utf-8") + except OSError: + logger.exception("Failed to write last-seen version:") + + +def _write_last_seen_version_if_absent(version: str) -> bool: + """Create the last-seen marker only if another process has not done so.""" + try: + with LAST_SEEN_VERSION_FILE.open("x", encoding="utf-8") as f: + f.write(version) + return True + except FileExistsError: + return False + except OSError: + logger.exception("Failed to create last-seen version:") + return False + + +def _cached_update_available() -> str | None: + """Return a newer cached release version after shared non-session filters. + + This intentionally ignores the per-session skip flag; callers that surface + transient toasts apply that suppression separately, while the welcome banner + uses this result as a session-persistent reminder. + """ + from pythinker_code.constant import VERSION as current_version + + if _auto_update_disabled() or _is_running_from_source_checkout(): + return None + cached = _read_latest_version_cache() + if not cached: + return None + if semver_tuple(cached) <= semver_tuple(current_version): + return None + if _read_dismissed_version() == cached: + return None + return cached + + +def welcome_update_target() -> str | None: + """Cached newer release version for the welcome-banner chip, or None. + + Unlike ``pending_update_notice`` this does not suppress when the user + chose 'Skip this session' on the startup modal — the banner chip is the + session-persistent reminder of that skip. + """ + return _cached_update_available() + + +def consume_whats_new() -> str | None: + """Return the current version string on first launch after an upgrade, else None. + + Side-effect: records the current version as 'last seen' so subsequent + launches in the same installation return None. No disk write in steady + state (last_seen == current). First-ever launch writes the baseline and + returns None so existing installs upgrading onto this feature see nothing + until the *next* upgrade. + """ + if _is_running_from_source_checkout(): + return None + + from pythinker_code.constant import VERSION as current_version + + last_seen = _read_last_seen_version() + if last_seen is None: + # First launch — establish baseline, show nothing. Use exclusive create + # so concurrent first launches do not both truncate/write the marker. + if not _write_last_seen_version_if_absent(current_version): + last_seen = _read_last_seen_version() + if last_seen is None: + # Repair an empty/corrupt marker left by a crashed concurrent writer. + _write_last_seen_version(current_version) + return None + if last_seen == current_version: + return None + # Upgraded since last launch. + _write_last_seen_version(current_version) + return current_version + + async def refresh_update_cache_if_due() -> UpdateResult | None: """Refresh the cached latest native release when the startup throttle allows it.""" return await _refresh_update_cache(force=False) @@ -442,18 +535,14 @@ def pending_update_notice() -> str | None: Reads only the cached latest version (no network). This is used by the background refresher after the pre-start prompt has had first chance to interrupt the session. Suppressed for source checkouts, disabled - auto-update, and per-version dismissals. + auto-update, per-version dismissals, and session-level skips. """ from pythinker_code.constant import VERSION as current_version - if _auto_update_disabled() or _is_running_from_source_checkout(): - return None - cached = _read_latest_version_cache() + cached = _cached_update_available() if not cached: return None - if semver_tuple(cached) <= semver_tuple(current_version): - return None - if _read_dismissed_version() == cached or cached == _skipped_version_this_session: + if cached == _skipped_version_this_session: return None return f"Update available: {current_version} → {cached}. Run /update to install." diff --git a/tests/ui_and_conv/test_shell_feedback_slash.py b/tests/ui_and_conv/test_shell_feedback_slash.py index e5cfd132..681502c5 100644 --- a/tests/ui_and_conv/test_shell_feedback_slash.py +++ b/tests/ui_and_conv/test_shell_feedback_slash.py @@ -3,11 +3,30 @@ from __future__ import annotations from collections.abc import Awaitable -from unittest.mock import Mock +from pathlib import Path +from typing import cast +from unittest.mock import AsyncMock, Mock +from pythinker_core.message import Message, ToolCall + +from pythinker_code.feedback import ( + FeedbackSubmission, + build_feedback_issue_url, + feedback_summary, + parse_feedback_args, + redact_text, + submit_feedback_payload, +) from pythinker_code.ui.shell import slash as shell_slash from pythinker_code.ui.shell.slash import registry as shell_slash_registry from pythinker_code.ui.shell.slash import shell_mode_registry +from pythinker_code.wire.types import TextPart, ThinkPart + + +async def _run_feedback(app: object, args: str) -> None: + result = shell_slash.feedback(app, args) # pyright: ignore[reportArgumentType] + if result is not None: + await cast(Awaitable[None], result) class TestFeedbackRegistration: @@ -21,41 +40,297 @@ def test_registered_in_shell_mode_registry(self) -> None: assert cmd is not None -class TestFeedbackOpensIssue: - def test_opens_new_issue_url(self, monkeypatch) -> None: +class TestFeedbackFallback: + async def test_opens_new_issue_url_when_no_soul(self, monkeypatch) -> None: open_mock = Mock(return_value=True) monkeypatch.setattr("pythinker_code.utils.term.open_url_in_browser", open_mock) monkeypatch.setattr(shell_slash.console, "print", Mock()) - shell = Mock() - ret = shell_slash.feedback(shell, "") - assert not isinstance(ret, Awaitable) + await _run_feedback(Mock(), "bug broken thing") open_mock.assert_called_once() url = open_mock.call_args.args[0] assert "TechMatrix-labs/pythinker-code" in url assert "new" in url - def test_prints_success_when_browser_opens(self, monkeypatch) -> None: + async def test_prints_success_when_browser_opens(self, monkeypatch) -> None: monkeypatch.setattr( "pythinker_code.utils.term.open_url_in_browser", Mock(return_value=True) ) print_mock = Mock() monkeypatch.setattr(shell_slash.console, "print", print_mock) - shell_slash.feedback(Mock(), "") + await _run_feedback(Mock(), "feature add thing") output = " ".join(str(c) for c in print_mock.call_args_list) assert "Opening" in output or "browser" in output.lower() - def test_prints_url_when_browser_fails(self, monkeypatch) -> None: + async def test_prints_url_when_browser_fails(self, monkeypatch) -> None: monkeypatch.setattr( "pythinker_code.utils.term.open_url_in_browser", Mock(return_value=False) ) print_mock = Mock() monkeypatch.setattr(shell_slash.console, "print", print_mock) - shell_slash.feedback(Mock(), "") + await _run_feedback(Mock(), "ux confusing prompt") output = " ".join(str(c) for c in print_mock.call_args_list) assert "TechMatrix-labs/pythinker-code" in output + + async def test_invalid_args_without_soul_still_offer_github_fallback(self, monkeypatch) -> None: + open_mock = Mock(return_value=True) + monkeypatch.setattr("pythinker_code.utils.term.open_url_in_browser", open_mock) + print_mock = Mock() + monkeypatch.setattr(shell_slash.console, "print", print_mock) + + await _run_feedback(Mock(), "--unknown-option") + + open_mock.assert_called_once() + output = " ".join(str(c) for c in print_mock.call_args_list) + assert "Unknown /feedback option" in output + assert "Opening GitHub feedback" in output + + +class TestFeedbackSubmission: + async def test_submits_structured_payload(self, tmp_path: Path, monkeypatch) -> None: + from pythinker_code.soul.pythinkersoul import PythinkerSoul + + soul = Mock(spec=PythinkerSoul) + soul.runtime.session.id = "sess-123" + soul.runtime.session.title = "Feedback task" + soul.runtime.session.work_dir = tmp_path + soul.runtime.session.subagents_dir = tmp_path / "subagents" + soul.runtime.session.subagents_dir.mkdir() + soul.runtime.role = "root" + soul.runtime.config.feedback.github_repo = "TechMatrix-labs/pythinker-code" + soul.name = "default" + soul.context.history = [ + Message(role="user", content=[TextPart(text="please fix this")]), + Message( + role="assistant", content=[ThinkPart(think="hidden"), TextPart(text="I can help")] + ), + Message( + role="assistant", + content=[], + tool_calls=[ + ToolCall( + id="call-1", + function=ToolCall.FunctionBody( + name="Bash", arguments='{"command":"pytest"}' + ), + ) + ], + ), + ] + + app = Mock() + app.soul = soul + monkeypatch.setattr(shell_slash, "_feedback_destination", lambda _soul: ("https://fb", {})) + submit_mock = AsyncMock( + return_value=FeedbackSubmission(number=42, html_url="https://issue/42") + ) + monkeypatch.setattr("pythinker_code.feedback.submit_feedback_payload", submit_mock) + monkeypatch.setattr("pythinker_code.feedback.current_model_key", lambda _soul: "test/model") + print_mock = Mock() + monkeypatch.setattr(shell_slash.console, "print", print_mock) + + await _run_feedback(app, "bug --yes command failed") + + submit_mock.assert_awaited_once() + submit_call = submit_mock.await_args + assert submit_call is not None + payload = submit_call.args[2] + assert payload["type"] == "bug" + assert payload["content"] == "command failed" + assert payload["session_id"] == "sess-123" + assert payload["privacy"]["redacted"] is True + assert payload["privacy"]["included_diff"] is False + assert payload["context"]["last_messages"][-1]["text"] == "I can help" + assert "hidden" not in str(payload) + assert payload["context"]["tool_calls"][-1]["name"] == "Bash" + + async def test_prints_follow_up_issue_url_when_endpoint_returns_no_link( + self, monkeypatch + ) -> None: + from pythinker_code.soul.pythinkersoul import PythinkerSoul + + soul = Mock(spec=PythinkerSoul) + soul.runtime.session.id = "sess-123" + soul.runtime.config.feedback.github_repo = "TechMatrix-labs/pythinker-code" + app = Mock() + app.soul = soul + payload = { + "type": "bug", + "content": "command failed", + "privacy": {}, + "context": {}, + "repo": {}, + } + monkeypatch.setattr( + "pythinker_code.feedback.build_feedback_payload", AsyncMock(return_value=payload) + ) + monkeypatch.setattr(shell_slash, "_feedback_destination", lambda _soul: ("https://fb", {})) + monkeypatch.setattr( + "pythinker_code.feedback.submit_feedback_payload", + AsyncMock(return_value=FeedbackSubmission(number=None, html_url=None)), + ) + print_mock = Mock() + monkeypatch.setattr(shell_slash.console, "print", print_mock) + + await _run_feedback(app, "bug --yes command failed") + + output = " ".join(str(c) for c in print_mock.call_args_list) + assert "No report link was returned" in output + assert "github.com/TechMatrix-labs/pythinker-code" in output + + async def test_prompts_before_submitting_by_default(self, monkeypatch) -> None: + from pythinker_code.soul.pythinkersoul import PythinkerSoul + + class RejectPromptSession: + @classmethod + def __class_getitem__(cls, _item: object) -> type[RejectPromptSession]: + return cls + + async def prompt_async(self, *_args: object, **_kwargs: object) -> str: + return "n" + + app = Mock() + app.soul = Mock(spec=PythinkerSoul) + monkeypatch.setattr( + "pythinker_code.feedback.build_feedback_payload", + AsyncMock(return_value={"privacy": {}, "context": {}, "repo": {}}), + ) + submit_mock = AsyncMock() + monkeypatch.setattr("pythinker_code.feedback.submit_feedback_payload", submit_mock) + monkeypatch.setattr("prompt_toolkit.PromptSession", RejectPromptSession) + print_mock = Mock() + monkeypatch.setattr(shell_slash.console, "print", print_mock) + + await _run_feedback(app, "bug command failed") + + submit_mock.assert_not_awaited() + output = " ".join(str(c) for c in print_mock.call_args_list) + assert "Feedback cancelled" in output + + +class TestFeedbackHelpers: + def test_parse_feedback_type_and_flags(self) -> None: + parsed = parse_feedback_args("bug --include-diff --yes broken tests") + assert not isinstance(parsed, str) + assert parsed.kind == "bug" + assert parsed.include_diff is True + assert parsed.yes is True + assert parsed.message == "broken tests" + + async def test_submit_feedback_payload_accepts_empty_success_body(self, monkeypatch) -> None: + class FakeResponse: + status = 200 + + async def __aenter__(self) -> FakeResponse: + return self + + async def __aexit__(self, *_args: object) -> None: + return None + + async def json(self, *_args: object, **_kwargs: object) -> object: + raise ValueError("empty body") + + class FakeSession: + async def __aenter__(self) -> FakeSession: + return self + + async def __aexit__(self, *_args: object) -> None: + return None + + def post(self, *_args: object, **_kwargs: object) -> FakeResponse: + return FakeResponse() + + monkeypatch.setattr("pythinker_code.feedback.new_client_session", FakeSession) + + submission = await submit_feedback_payload("https://feedback", {}, {"content": "hi"}) + + assert submission.number is None + assert submission.html_url is None + + def test_feedback_issue_url_uses_compact_body_for_large_payload(self) -> None: + payload = { + "type": "bug", + "content": "problem " * 600, + "session_id": "sess-123", + "client": { + "version": "1.2.3", + "os": "Linux", + "python": "3.14", + "model": "test/model", + }, + "repo": { + "branch": "feature/feedback", + "head": "abc1234", + "dirty": True, + "diff": "+secret diff\n" * 10_000, + }, + "context": { + "last_messages": [{"role": "user", "text": "message " * 500}], + "tool_calls": [{"name": "Bash", "hint": "pytest"}], + }, + "privacy": {"redacted": True, "included_diff": True}, + } + + url = build_feedback_issue_url(payload) + + assert len(url) < 8_000 + assert "+secret diff" not in url + assert "Patch+diff+omitted" in url + + def test_feedback_summary_shows_default_privacy_exclusions(self) -> None: + summary = feedback_summary( + { + "privacy": { + "included_diff": False, + "included_transcript": False, + "included_tool_details": False, + }, + "context": {}, + "repo": {}, + } + ) + + assert "✗ patch diff" in summary + assert "✗ extended transcript" in summary + assert "✗ detailed tool args/results" in summary + assert "best-effort secret/path redaction" in summary + + def test_redacts_common_secrets_and_home_path(self) -> None: + text = f"Authorization: Bearer ghp_abcdefghijklmnopqrstuvwxyz123456 {Path.home()}/repo" + + redacted = redact_text(text) + + assert "ghp_" not in redacted + assert str(Path.home()) not in redacted + assert " None: + text = " ".join( + [ + "".join(["sk-", "ant", "-api03-", "abcdefghijklmnopqrstuvwxyz0123456789"]), + "".join(["sk-", "proj", "-", "abcdefghijklmnopqrstuvwxyz0123456789"]), + "".join(["xo", "xb-", "1234567890", "-", "abcdefghijklmnop"]), + "".join(["AI", "za", "SyAbCdEfGhIjKlMnOpQrStUvWxYz012345678"]), + ".".join( + [ + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9", + "eyJzdWIiOiIxMjM0NTY3ODkwIn0", + "signature1234567890", + ] + ), + ] + ) + + redacted = redact_text(text) + + assert "sk-ant" not in redacted + assert "sk-proj" not in redacted + assert "xoxb" not in redacted + assert "AIza" not in redacted + assert "eyJ" not in redacted + assert redacted.count("= 5 diff --git a/tests/ui_and_conv/test_shell_update.py b/tests/ui_and_conv/test_shell_update.py index bf018b9f..7c87e3c8 100644 --- a/tests/ui_and_conv/test_shell_update.py +++ b/tests/ui_and_conv/test_shell_update.py @@ -946,3 +946,116 @@ class _PromptReached(Exception): prompt_mock.assert_awaited_once() auto_update_mock.assert_not_called() + + +# --------------------------------------------------------------------------- +# welcome_update_target and consume_whats_new +# --------------------------------------------------------------------------- + + +def test_welcome_update_target_returns_newer_cached_version(monkeypatch, tmp_path): + latest_file = tmp_path / "latest.txt" + latest_file.write_text("99.0.0", encoding="utf-8") + dismissed_file = tmp_path / "dismissed.txt" + + monkeypatch.setattr(update, "LATEST_VERSION_FILE", latest_file) + monkeypatch.setattr(update, "DISMISSED_VERSION_FILE", dismissed_file) + monkeypatch.setattr(update, "_auto_update_disabled", lambda: False) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: False) + + result = update.welcome_update_target() + assert result == "99.0.0" + + +def test_welcome_update_target_not_suppressed_by_session_skip(monkeypatch, tmp_path): + latest_file = tmp_path / "latest.txt" + latest_file.write_text("99.0.0", encoding="utf-8") + dismissed_file = tmp_path / "dismissed.txt" + + monkeypatch.setattr(update, "LATEST_VERSION_FILE", latest_file) + monkeypatch.setattr(update, "DISMISSED_VERSION_FILE", dismissed_file) + monkeypatch.setattr(update, "_auto_update_disabled", lambda: False) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: False) + # Simulate that the user chose "Skip this session" on the modal. + monkeypatch.setattr(update, "_skipped_version_this_session", "99.0.0") + + # welcome_update_target does NOT suppress session-skips (that's its purpose). + assert update.welcome_update_target() == "99.0.0" + + +def test_welcome_update_target_suppressed_by_dismiss(monkeypatch, tmp_path): + latest_file = tmp_path / "latest.txt" + latest_file.write_text("99.0.0", encoding="utf-8") + dismissed_file = tmp_path / "dismissed.txt" + dismissed_file.write_text("99.0.0", encoding="utf-8") + + monkeypatch.setattr(update, "LATEST_VERSION_FILE", latest_file) + monkeypatch.setattr(update, "DISMISSED_VERSION_FILE", dismissed_file) + monkeypatch.setattr(update, "_auto_update_disabled", lambda: False) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: False) + + assert update.welcome_update_target() is None + + +def test_welcome_update_target_suppressed_for_source_checkout(monkeypatch, tmp_path): + latest_file = tmp_path / "latest.txt" + latest_file.write_text("99.0.0", encoding="utf-8") + + monkeypatch.setattr(update, "LATEST_VERSION_FILE", latest_file) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: True) + + assert update.welcome_update_target() is None + + +def test_consume_whats_new_baseline_on_first_launch(monkeypatch, tmp_path): + from pythinker_code.constant import VERSION as current_version + + last_seen_file = tmp_path / "last_seen.txt" + + monkeypatch.setattr(update, "LAST_SEEN_VERSION_FILE", last_seen_file) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: False) + + # First-ever launch: no file → write the current version as baseline, return None. + result = update.consume_whats_new() + assert result is None + assert last_seen_file.read_text(encoding="utf-8").strip() == current_version + + +def test_consume_whats_new_returns_version_after_upgrade(monkeypatch, tmp_path): + from pythinker_code.constant import VERSION as current_version + + last_seen_file = tmp_path / "last_seen.txt" + last_seen_file.write_text("0.0.1", encoding="utf-8") # older than current + + monkeypatch.setattr(update, "LAST_SEEN_VERSION_FILE", last_seen_file) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: False) + + result = update.consume_whats_new() + assert result == current_version + assert last_seen_file.read_text(encoding="utf-8").strip() == current_version + + +def test_consume_whats_new_no_disk_write_in_steady_state(monkeypatch, tmp_path): + from pythinker_code.constant import VERSION as current_version + + last_seen_file = tmp_path / "last_seen.txt" + last_seen_file.write_text(current_version, encoding="utf-8") + mtime_before = last_seen_file.stat().st_mtime + + monkeypatch.setattr(update, "LAST_SEEN_VERSION_FILE", last_seen_file) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: False) + + result = update.consume_whats_new() + assert result is None + # File must not have been rewritten (mtime unchanged). + assert last_seen_file.stat().st_mtime == mtime_before + + +def test_consume_whats_new_suppressed_for_source_checkout(monkeypatch, tmp_path): + last_seen_file = tmp_path / "last_seen.txt" + last_seen_file.write_text("0.0.1", encoding="utf-8") + + monkeypatch.setattr(update, "LAST_SEEN_VERSION_FILE", last_seen_file) + monkeypatch.setattr(update, "_is_running_from_source_checkout", lambda: True) + + assert update.consume_whats_new() is None diff --git a/tests/ui_and_conv/test_shell_welcome_info.py b/tests/ui_and_conv/test_shell_welcome_info.py index 6138ba0b..ffd2ab70 100644 --- a/tests/ui_and_conv/test_shell_welcome_info.py +++ b/tests/ui_and_conv/test_shell_welcome_info.py @@ -1,4 +1,5 @@ from rich.console import Console +from rich.text import Text from pythinker_code.ui import shell as shell_module @@ -23,3 +24,48 @@ def test_directory_label_uses_brand_info_token(): set_active_theme("dark") style = _value_style_for_label("Directory", WelcomeInfoItem.Level.INFO) assert get_tui_tokens("dark").info in style # "#AFE3F1" + + +def test_welcome_banner_chip_shown_in_output(monkeypatch): + console = Console(record=True, width=120, color_system=None) + monkeypatch.setattr(shell_module, "console", console) + monkeypatch.setattr(shell_module, "get_version", lambda: "9.9.9") + + chip = Text("↑ Update available — v1.0.0 · /update") + shell_module._print_welcome_info("Pythinker Code", [], banner=chip) + + output = console.export_text() + assert "Update available" in output + assert "/update" in output + assert "Welcome to Pythinker" in output + + +def test_welcome_banner_chip_update_wins_over_whats_new(monkeypatch): + monkeypatch.setattr(shell_module, "consume_whats_new", lambda: "0.25.0") + monkeypatch.setattr(shell_module, "welcome_update_target", lambda: "0.26.0") + + chip = shell_module._welcome_banner_chip() + + assert chip is not None + text = chip.plain + assert "Update available" in text + assert "0.26.0" in text + assert "What's new" not in text + + +def test_welcome_banner_no_chip_unchanged(monkeypatch): + console_with = Console(record=True, width=120, color_system=None) + console_without = Console(record=True, width=120, color_system=None) + monkeypatch.setattr(shell_module, "get_version", lambda: "0.26.0") + + monkeypatch.setattr(shell_module, "console", console_without) + shell_module._print_welcome_info("Pythinker Code", []) + out_without = console_without.export_text() + + monkeypatch.setattr(shell_module, "console", console_with) + shell_module._print_welcome_info("Pythinker Code", [], banner=None) + out_with = console_with.export_text() + + # Both paths produce the same output when banner is None. + assert out_without == out_with + assert "Welcome to Pythinker" in out_without