From bffbc8e96f817ebb9d1b4a97730e442ce0ef23e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 30 Jun 2026 00:25:34 +0100 Subject: [PATCH] ci: add harness file linter (#1549) * ci: add harness file linter * ci: address harness linter review feedback * ci: cover codex harness config linting * ci: tighten harness lint opt-ins * ci: expand harness lint coverage * ci: address harness linter review feedback * ci: fix harness linter review followups * ci: lint claude settings harness config * ci: tighten harness linter coverage * ci: fail malformed harness toml --------- Co-authored-by: root --- .github/scripts/lint-harness-files.py | 601 ++++++++++++++++++ .github/workflows/lint-harness-files.yml | 81 +++ .pre-commit-config.yaml | 9 + Makefile | 8 +- .../task/service/service_tasks_stop_test.go | 10 +- .../task/dockview-desktop-layout.test.ts | 4 +- .../task/dockview-desktop-layout.tsx | 314 +-------- .../task/dockview-panel-content.tsx | 276 ++++++++ .../task/dockview-review-dialog.tsx | 29 + scripts/lint-harness-files.test.py | 350 ++++++++++ 10 files changed, 1363 insertions(+), 319 deletions(-) create mode 100644 .github/scripts/lint-harness-files.py create mode 100644 .github/workflows/lint-harness-files.yml create mode 100644 apps/web/components/task/dockview-panel-content.tsx create mode 100644 apps/web/components/task/dockview-review-dialog.tsx create mode 100644 scripts/lint-harness-files.test.py diff --git a/.github/scripts/lint-harness-files.py b/.github/scripts/lint-harness-files.py new file mode 100644 index 0000000000..244255bd06 --- /dev/null +++ b/.github/scripts/lint-harness-files.py @@ -0,0 +1,601 @@ +#!/usr/bin/env python3 +"""Lint Kandev harness files. + +Usage: + python3 .github/scripts/lint-harness-files.py --all + python3 .github/scripts/lint-harness-files.py AGENTS.md .agents/skills/fix/SKILL.md +""" + +import argparse +import os +import re +import subprocess +import sys +from pathlib import Path + +try: + import tomllib +except ModuleNotFoundError: # Python < 3.11 + try: + import tomli as tomllib + except ModuleNotFoundError: + tomllib = None + + +DEFAULT_RULES_BY_KIND = { + "agent": { + "agent-line-limit": {"limit": 300}, + }, + "skill": { + "skill-line-limit": {"limit": 500}, + "description-word-limit": {"limit": 600}, + }, + "reference": { + "reference-line-limit": {"limit": 400}, + }, + "role-agent": { + "role-agent-line-limit": {"limit": 300}, + "description-word-limit": {"limit": 600}, + }, + "command": { + "command-line-limit": {"limit": 300}, + "description-word-limit": {"limit": 600}, + }, + "cursor-rule": { + "cursor-rule-line-limit": {"limit": 300}, + "description-word-limit": {"limit": 600}, + }, + "config": { + "config-line-limit": {"limit": 300}, + }, +} + +LINE_LIMIT_RULES = { + "agent-line-limit", + "skill-line-limit", + "reference-line-limit", + "role-agent-line-limit", + "command-line-limit", + "cursor-rule-line-limit", + "config-line-limit", +} + +OPT_IN_RULES = { + "description-when", + "no-at-dot-slash", + "no-emoji", + "no-em-dash", +} + +OPT_IN_RULES_BY_KIND = { + "description-when": {"skill", "role-agent", "command", "cursor-rule"}, + "no-at-dot-slash": set(DEFAULT_RULES_BY_KIND), + "no-emoji": set(DEFAULT_RULES_BY_KIND), + "no-em-dash": set(DEFAULT_RULES_BY_KIND), +} + +YAML_BLOCK_SCALAR_MARKERS = {">", "|", ">-", "|-", ">+", "|+"} + +EMOJI_RANGES = ( + (0x2600, 0x27BF), + (0x1F000, 0x1FAFF), +) + + +class Violation: + def __init__(self, rule: str, path: Path, line: int, msg: str) -> None: + self.rule = rule + self.path = path + self.line = line + self.msg = msg + + def __repr__(self) -> str: + return f"Violation(rule={self.rule!r}, path={str(self.path)!r}, line={self.line!r}, msg={self.msg!r})" + + +def main() -> int: + parser = argparse.ArgumentParser(description="Lint Kandev harness files.") + parser.add_argument("targets", nargs="*", help="Files or directories to lint") + parser.add_argument("--all", action="store_true", help="Lint every tracked harness file") + parser.add_argument("--disable", action="append", default=[], help="Disable one rule by id") + parser.add_argument("--enable", action="append", default=[], help="Enable one opt-in rule by id") + args = parser.parse_args() + + if not args.all and not args.targets: + parser.print_usage(sys.stderr) + return 2 + + cwd = Path.cwd() + try: + paths = discover_all(cwd) if args.all else discover_targets([Path(target) for target in args.targets], cwd) + except RuntimeError as exc: + print(str(exc), file=sys.stderr) + return 2 + + disabled = set(args.disable) + enabled = set(args.enable) + unknown_enabled = sorted(enabled - OPT_IN_RULES) + if unknown_enabled: + print(f"unknown opt-in rule(s): {', '.join(unknown_enabled)}", file=sys.stderr) + return 2 + violations = [] + linted = 0 + + try: + for path in paths: + kind = classify_path(path) + if kind == "unknown": + continue + linted += 1 + rules = dict(DEFAULT_RULES_BY_KIND.get(kind, {})) + for rule in enabled: + if kind not in OPT_IN_RULES_BY_KIND[rule]: + continue + rules.setdefault(rule, {}) + violations.extend(lint_path(path, kind, rules=rules, disabled=disabled)) + except RuntimeError as exc: + print(str(exc), file=sys.stderr) + return 2 + + for violation in violations: + print(format_violation(violation, cwd)) + if os.environ.get("GITHUB_ACTIONS") == "true": + print(format_github_annotation(violation, cwd)) + + if violations: + print(f"\n{len(violations)} harness lint violation(s) found.", file=sys.stderr) + return 1 + + print(f"All {linted} harness file(s) passed.") + return 0 + + +def discover_all(cwd: Path) -> list[Path]: + result = subprocess.run( + ["git", "ls-files", "-z"], + cwd=cwd, + text=False, + capture_output=True, + check=False, + ) + if result.returncode != 0: + stderr = result.stderr.decode("utf-8", errors="replace").strip() + raise RuntimeError(stderr or "git ls-files failed") + names = [name.decode("utf-8", errors="replace") for name in result.stdout.split(b"\0") if name] + return [cwd / name for name in names] + + +def discover_targets(targets: list[Path], cwd: Path) -> list[Path]: + paths = [] + for target in targets: + path = target if target.is_absolute() else cwd / target + if not path.exists(): + continue + if path.is_dir() and not path.is_symlink(): + paths.extend(walk_without_symlink_dirs(path)) + continue + paths.append(path) + return sorted(paths) + + +def walk_without_symlink_dirs(root: Path) -> list[Path]: + paths = [] + for dirpath, dirnames, filenames in os.walk(root, followlinks=False): + current = Path(dirpath) + dirnames[:] = [name for name in dirnames if not (current / name).is_symlink()] + for filename in filenames: + paths.append(current / filename) + return paths + + +def classify_path(path: Path) -> str: + name = path.name + if name in {"AGENTS.md", "CLAUDE.md", ".cursorrules"}: + return "agent" + + parts = normalized_parts(path) + if ( + has_subpath(parts, [".agents", "skills"]) + or has_subpath(parts, [".augment", "skills"]) + or has_subpath(parts, [".claude", "skills"]) + or has_subpath(parts, [".cursor", "skills"]) + or has_subpath(parts, [".opencode", "skills"]) + ): + if name == "SKILL.md": + return "skill" + if name.endswith(".md"): + return "reference" + + if has_subpath(parts, ["apps", "backend", "internal", "office", "configloader", "skills"]): + if name == "SKILL.md": + return "skill" + if name.endswith(".md"): + return "reference" + + if has_subpath(parts, [".agents", "agents"]) and name.endswith(".md"): + return "role-agent" + if has_subpath(parts, [".opencode", "agents"]) and name.endswith(".md"): + return "role-agent" + if has_subpath(parts, [".claude", "agents"]) and name.endswith(".md"): + return "role-agent" + if has_subpath(parts, [".codex", "agents"]) and name.endswith(".toml"): + return "role-agent" + if has_subpath(parts, [".codex"]) and name == "config.toml": + return "config" + if has_subpath(parts, [".claude"]) and name == "settings.json": + return "config" + if has_subpath(parts, [".claude", "commands"]) and name.endswith(".md"): + return "command" + if has_subpath(parts, [".claude", "rules"]) and name.endswith(".md"): + return "cursor-rule" + if has_subpath(parts, [".cursor", "rules"]) and name.endswith(".mdc"): + return "cursor-rule" + + return "unknown" + + +def normalized_parts(path: Path) -> list[str]: + return [part for part in path.as_posix().split("/") if part not in {"", "."}] + + +def has_subpath(parts: list[str], needle: list[str]) -> bool: + if len(parts) < len(needle): + return False + for index in range(0, len(parts) - len(needle) + 1): + if parts[index : index + len(needle)] == needle: + return True + return False + + +def lint_path( + path: Path, + kind: str | None = None, + rules: dict[str, dict[str, int]] | None = None, + disabled: set[str] | None = None, +) -> list[Violation]: + kind = kind or classify_path(path) + if kind == "unknown": + return [] + rules = rules if rules is not None else DEFAULT_RULES_BY_KIND.get(kind, {}) + disabled = disabled or set() + text = path.read_text(encoding="utf-8") + lines = text.splitlines() + + violations = [] + for rule, options in rules.items(): + if rule in disabled: + continue + if rule in OPT_IN_RULES_BY_KIND and kind not in OPT_IN_RULES_BY_KIND[rule]: + continue + if rule in LINE_LIMIT_RULES: + violations.extend(check_line_limit(path, rule, lines, int(options.get("limit", 0)))) + elif rule == "description-word-limit": + violations.extend(check_description_word_limit(path, text, int(options.get("limit", 0)))) + elif rule == "no-emoji": + violations.extend(check_no_emoji(path, lines)) + elif rule == "no-em-dash": + violations.extend(check_no_em_dash(path, lines)) + elif rule == "no-at-dot-slash": + violations.extend(check_no_at_dot_slash(path, lines)) + elif rule == "description-when": + violations.extend(check_description_when(path, text)) + return violations + + +def check_line_limit(path: Path, rule: str, lines: list[str], limit: int) -> list[Violation]: + if limit <= 0: + return [] + count = len(lines) + if count <= limit: + return [] + return [Violation(rule, path, 0, line_limit_message(rule, count, limit))] + + +def line_limit_message(rule: str, count: int, limit: int) -> str: + prefix = f"file is {count} lines (limit {limit}, over by {count - limit})." + target = f"target: {limit} lines or fewer." + if rule == "agent-line-limit": + return ( + f"{prefix} why: this file is reloaded into the agent's context on every turn; every line here is paid for " + "in every conversation. fix: trim down to the essential rules and routing signals. move detailed examples, " + f"command catalogues, or reference material into a sibling file (e.g. REFERENCE.md, EXAMPLES.md) and link " + f"to it from this file. {target}" + ) + if rule == "skill-line-limit": + return ( + f"{prefix} why: when this skill triggers, the body loads into context and competes with the running " + "conversation. fix: keep SKILL.md focused on (1) when to trigger and (2) the core steps. move long examples, " + f"command catalogues, troubleshooting, or reference material into sibling files (REFERENCE.md, EXAMPLES.md, " + f"examples/*.md) and link them from SKILL.md. {target}" + ) + if rule == "reference-line-limit": + return ( + f"{prefix} why: oversize reference files signal a monolithic skill that is hard to navigate and slow to load " + "on demand. fix: split this file into multiple focused files in the same skill directory (one per topic) " + f"and update the SKILL.md links. {target}" + ) + if rule == "role-agent-line-limit": + return ( + f"{prefix} why: agent definitions are loaded into model/router context when selecting or spawning specialist " + "agents. fix: keep the file focused on role, tool constraints, and workflow; move long examples or command " + f"catalogues into referenced files. {target}" + ) + if rule == "command-line-limit": + return ( + f"{prefix} why: slash-command prompts are loaded when invoked and compete with the active task context. " + "fix: keep the command compact and move long examples, command catalogues, or rationale into referenced " + f"files. {target}" + ) + if rule == "cursor-rule-line-limit": + return ( + f"{prefix} why: Cursor rules are routing and style context that should stay concise. fix: move detailed " + f"examples or long reference material into sibling files and link to them. {target}" + ) + if rule == "config-line-limit": + return ( + f"{prefix} why: agent config files are read when spawning or routing agent runs, and bloated config obscures " + "the active model, permission, and tool choices. fix: keep config files to the settings the runtime needs; " + f"move rationale, examples, or setup notes into a referenced markdown file. {target}" + ) + return f"{prefix} {target}" + + +def check_description_word_limit(path: Path, text: str, limit: int) -> list[Violation]: + if limit <= 0: + return [] + description = extract_description(path, text) + if not description: + return [] + count = len(description.split()) + if count <= limit: + return [] + message = ( + f"description is {count} words (limit {limit}, over by {count - limit}). why: harness descriptions are loaded " + "into routing context before the body is selected, and long descriptions compete with discovery of sibling " + "artifacts. fix: keep the description to a single routing sentence ('Use when X. Do Y.') and move details, " + f"examples, command lists, or rationale into the body or a referenced file. target: {limit} words or fewer." + ) + return [Violation("description-word-limit", path, 1, message)] + + +def extract_description(path: Path, text: str) -> str | None: + if path.suffix == ".toml": + return extract_toml_description(text) + status, description = extract_yaml_description(text.splitlines()) + if status != "ok": + return None + return description + + +def extract_yaml_description(lines: list[str]) -> tuple[str, str | None]: + if not lines or lines[0].strip() != "---": + return "missing", None + close_index = None + for index in range(1, len(lines)): + if lines[index] == "---": + close_index = index + break + if close_index is None: + return "unclosed", None + + frontmatter = lines[1:close_index] + for index, line in enumerate(frontmatter): + if line.startswith((" ", "\t")): + continue + if not line.startswith("description:"): + continue + value = line[len("description:") :].strip() + chunks = [strip_yaml_scalar(value)] if value and value not in YAML_BLOCK_SCALAR_MARKERS else [] + for continuation in frontmatter[index + 1 :]: + if not continuation.startswith((" ", "\t")): + break + stripped = continuation.strip() + if stripped: + chunks.append(strip_yaml_scalar(stripped)) + return ("empty" if not chunks else "ok"), " ".join(chunks).strip() + return "ok", None + + +def strip_yaml_scalar(value: str) -> str: + value = value.strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: + return value[1:-1] + return value + + +def extract_toml_description(text: str) -> str | None: + if tomllib is None: + raise RuntimeError( + "TOML harness files require Python 3.11+ or the 'tomli' package. " + "Install tomli or run this linter with Python 3.11+." + ) + try: + data = tomllib.loads(text) + except tomllib.TOMLDecodeError as exc: + raise RuntimeError(f"invalid TOML in harness file: {exc}") from exc + description = data.get("description") + if not isinstance(description, str): + return None + return description.strip() + + +def check_no_emoji(path: Path, lines: list[str]) -> list[Violation]: + violations = [] + for line_number, line in enumerate(lines, start=1): + for column, char in enumerate(line, start=1): + if is_emoji(char): + message = ( + f'emoji "{char}" at column {column}. why: emojis spend context tokens with no semantic value for ' + "the model and are banned by repo style in harness guidance files. fix: remove the emoji entirely, " + "or replace it with a short text label (e.g. [OK], [WARN], [FAIL])." + ) + violations.append(Violation("no-emoji", path, line_number, message)) + break + return violations + + +def is_emoji(char: str) -> bool: + codepoint = ord(char) + return any(start <= codepoint <= end for start, end in EMOJI_RANGES) + + +def check_no_em_dash(path: Path, lines: list[str]) -> list[Violation]: + violations = [] + for line_number, line in enumerate(lines, start=1): + index = line.find("\u2014") + if index == -1: + continue + column = index + 1 + message = ( + f"em-dash (U+2014) at column {column}. why: em-dashes are a stylistic tell of AI-generated prose; repo " + "style requires plain ASCII dashes in harness files. fix: replace U+2014 with ' - ' (space-hyphen-space) " + "or rephrase the sentence to remove the dash. search for the literal em-dash character (U+2014) in this " + "file to locate every occurrence." + ) + violations.append(Violation("no-em-dash", path, line_number, message)) + return violations + + +def check_no_at_dot_slash(path: Path, lines: list[str]) -> list[Violation]: + violations = [] + for line_number, line in enumerate(lines, start=1): + index = line.find("@./") + if index == -1: + continue + column = index + 1 + message = ( + f"`@./` reference at column {column}. why: non-standard import/path pattern that confuses " + "harness loaders and is not documented behaviour. fix: drop the `@./` prefix. example: `@./SKILL.md` " + "becomes `./SKILL.md` or `SKILL.md`; `@./examples/foo.sh` becomes `examples/foo.sh`." + ) + violations.append(Violation("no-at-dot-slash", path, line_number, message)) + return violations + + +def check_description_when(path: Path, text: str) -> list[Violation]: + if path.suffix == ".toml": + return check_toml_description_when(path, text) + return check_yaml_description_when(path, text) + + +def check_yaml_description_when(path: Path, text: str) -> list[Violation]: + lines = text.splitlines() + status, description = extract_yaml_description(lines) + if status == "missing": + return [ + Violation( + "description-when", + path, + 1, + "file is missing YAML frontmatter (`--- ... ---` block). why: harness artifacts are discovered via " + "their frontmatter `name` and `description` fields; without a frontmatter block the router cannot " + "trigger the artifact. fix: add a YAML block at the very top of the file: `---` on its own line, then " + "`name: `, then `description: Use when . .`, " + "then a closing `---` line.", + ) + ] + if status == "unclosed": + return [ + Violation( + "description-when", + path, + 1, + "file frontmatter opens with `---` but never closes. why: a malformed frontmatter block is " + "unparseable and the artifact becomes invisible to the router. fix: add a `---` line on its own " + "(no leading whitespace) after the last frontmatter field to close the block.", + ) + ] + if status == "empty": + return [ + Violation( + "description-when", + path, + 1, + "file frontmatter has an empty `description:` field. why: the description is the primary signal the " + "router uses to decide whether this artifact applies to the user's request. fix: fill in " + "`description: Use when . .` between the opening and closing " + "--- fences.", + ) + ] + if not description: + return [ + Violation( + "description-when", + path, + 1, + "file frontmatter has no `description:` field. why: the description is the primary signal the " + "router uses to decide whether this artifact applies to the user's request. fix: add `description: " + "Use when . .` between the opening and closing `---` fences. " + "example: `description: Use when the user says 'create a branch' or references a CLIP-XXX ticket.`", + ) + ] + if not has_routing_phrase(description): + return [description_routing_violation(path)] + return [] + + +def check_toml_description_when(path: Path, text: str) -> list[Violation]: + description = extract_toml_description(text) + if not description: + return [ + Violation( + "description-when", + path, + 1, + 'agent TOML has no `description = "..."` field. why: the description is the primary router signal ' + "for selecting this specialist agent. fix: add a concise `description = \"Use when ...\"` entry with " + "concrete task types or user phrases.", + ) + ] + if not has_routing_phrase(description): + return [description_routing_violation(path)] + return [] + + +def has_routing_phrase(description: str) -> bool: + # Before enabling description-when by default, update artifacts that currently + # use looser phrasing like "Useful for ..." or incidental "... when ...". + return re.search(r"\b(use when|use for|trigger on|trigger when)\b", description, re.IGNORECASE) is not None + + +def description_routing_violation(path: Path) -> Violation: + return Violation( + "description-when", + path, + 1, + 'description lacks a routing phrase. why: the model needs an explicit trigger signal ("when" / ' + '"use for") to pick this skill over siblings; vague descriptions cause missed triggers or wrong routing. ' + 'fix: start the description with one of "Use when ...", "Use for ...", or "Trigger on ..." followed by ' + 'concrete user phrases, file paths, or task types. example: "Use when the user says \'fix the bug\' or ' + '\'X is broken\'."', + ) + + +def format_violation(violation: Violation, cwd: Path) -> str: + path = display_path(violation.path, cwd) + if violation.line > 0: + return f"{path}:{violation.line}: [{violation.rule}] {violation.msg}" + return f"{path}: [{violation.rule}] {violation.msg}" + + +def format_github_annotation(violation: Violation, cwd: Path) -> str: + path = display_path(violation.path, cwd) + location = f"file={path}" + if violation.line > 0: + location += f",line={violation.line}" + return f"::error {location}::{escape_github_message(f'[{violation.rule}] {violation.msg}')}" + + +def display_path(path: Path, cwd: Path) -> str: + try: + return path.resolve().relative_to(cwd.resolve()).as_posix() + except ValueError: + return path.as_posix() + + +def escape_github_message(message: str) -> str: + return message.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/lint-harness-files.yml b/.github/workflows/lint-harness-files.yml new file mode 100644 index 0000000000..7a10d491cf --- /dev/null +++ b/.github/workflows/lint-harness-files.yml @@ -0,0 +1,81 @@ +name: Lint Harness Files + +on: + push: + branches: [main] + paths: + - ".github/workflows/lint-harness-files.yml" + - ".github/scripts/lint-harness-files.py" + - "scripts/lint-harness-files.test.py" + - ".pre-commit-config.yaml" + - "AGENTS.md" + - "CLAUDE.md" + - "**/AGENTS.md" + - "**/CLAUDE.md" + - "**/.cursorrules" + - ".agents/skills/**/*.md" + - ".augment/skills/**/*.md" + - ".claude/skills/**/*.md" + - ".cursor/skills/**/*.md" + - ".opencode/skills/**/*.md" + - "apps/backend/internal/office/configloader/skills/**/*.md" + - ".agents/agents/**/*.md" + - ".opencode/agents/**/*.md" + - ".claude/agents/**/*.md" + - ".codex/agents/**/*.toml" + - ".codex/config.toml" + - ".claude/settings.json" + - ".claude/rules/**/*.md" + - ".cursor/rules/**/*.mdc" + - ".claude/commands/**/*.md" + pull_request: + branches: [main] + paths: + - ".github/workflows/lint-harness-files.yml" + - ".github/scripts/lint-harness-files.py" + - "scripts/lint-harness-files.test.py" + - ".pre-commit-config.yaml" + - "AGENTS.md" + - "CLAUDE.md" + - "**/AGENTS.md" + - "**/CLAUDE.md" + - "**/.cursorrules" + - ".agents/skills/**/*.md" + - ".augment/skills/**/*.md" + - ".claude/skills/**/*.md" + - ".cursor/skills/**/*.md" + - ".opencode/skills/**/*.md" + - "apps/backend/internal/office/configloader/skills/**/*.md" + - ".agents/agents/**/*.md" + - ".opencode/agents/**/*.md" + - ".claude/agents/**/*.md" + - ".codex/agents/**/*.toml" + - ".codex/config.toml" + - ".claude/settings.json" + - ".claude/rules/**/*.md" + - ".cursor/rules/**/*.mdc" + - ".claude/commands/**/*.md" + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + lint: + name: Harness files pass lint + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false + + - name: Run linter tests + run: python3 scripts/lint-harness-files.test.py + + - name: Lint harness files + run: python3 .github/scripts/lint-harness-files.py --all diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 44dfd2f961..07658e843f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,6 +9,15 @@ repos: stages: [commit-msg] always_run: true + # Harness guidance hooks + - repo: local + hooks: + - id: harness-lint + name: Harness file lint + entry: python3 .github/scripts/lint-harness-files.py + language: system + pass_filenames: true + # Backend (Go) hooks - repo: local hooks: diff --git a/Makefile b/Makefile index 273d1366a7..5489cde90a 100644 --- a/Makefile +++ b/Makefile @@ -423,6 +423,7 @@ test-scripts: @bash scripts/pr-state.test.sh @bash scripts/opencode-code-review.test.sh @python3 scripts/opencode-code-review.test.py + @python3 scripts/lint-harness-files.test.py @bash scripts/release-desktop.test.sh .PHONY: test-e2e @@ -467,7 +468,7 @@ test-e2e-ci: # .PHONY: lint -lint: lint-backend lint-web +lint: lint-backend lint-web lint-harness @printf "\n$(GREEN)$(BOLD)✓ Linting complete!$(RESET)\n" .PHONY: lint-backend @@ -480,6 +481,11 @@ lint-web: @printf "$(CYAN)Linting web app...$(RESET)\n" @cd $(APPS_DIR) && $(PNPM) --filter @kandev/web lint +.PHONY: lint-harness +lint-harness: + @printf "$(CYAN)Linting harness files...$(RESET)\n" + @python3 .github/scripts/lint-harness-files.py --all + .PHONY: lint-format lint-format: @printf "$(CYAN)Checking formatting...$(RESET)\n" diff --git a/apps/backend/internal/task/service/service_tasks_stop_test.go b/apps/backend/internal/task/service/service_tasks_stop_test.go index f6fbd8764c..5357f8cfbd 100644 --- a/apps/backend/internal/task/service/service_tasks_stop_test.go +++ b/apps/backend/internal/task/service/service_tasks_stop_test.go @@ -51,10 +51,10 @@ func TestIsCleanableSessionState_IdleIncluded(t *testing.T) { // stubExecutors is a minimal repository.ExecutorRepository implementation for tests. type stubExecutors struct { repository.ExecutorRepository - runningByTaskID []*models.ExecutorRunning - runningByTaskErr error - runningBySession *models.ExecutorRunning - runningBySessErr error + runningByTaskID []*models.ExecutorRunning + runningByTaskErr error + runningBySession *models.ExecutorRunning + runningBySessErr error } func (s *stubExecutors) ListExecutorsRunningByTaskID(_ context.Context, _ string) ([]*models.ExecutorRunning, error) { @@ -150,7 +150,7 @@ type stubStopper struct { stopSessionErr error } -func (s *stubStopper) StopTask(_ context.Context, _, _ string, _ bool) error { return nil } +func (s *stubStopper) StopTask(_ context.Context, _, _ string, _ bool) error { return nil } func (s *stubStopper) StopExecution(_ context.Context, _, _ string, _ bool) error { return nil } func (s *stubStopper) StopSession(_ context.Context, _, _ string, _ bool) error { return s.stopSessionErr diff --git a/apps/web/components/task/dockview-desktop-layout.test.ts b/apps/web/components/task/dockview-desktop-layout.test.ts index 4adf55ca77..04a6fbc18e 100644 --- a/apps/web/components/task/dockview-desktop-layout.test.ts +++ b/apps/web/components/task/dockview-desktop-layout.test.ts @@ -1,12 +1,12 @@ import { describe, it, expect } from "vitest"; -import { CHAT_PANEL_FALLBACK_LABEL, resolveChatPanelTitle } from "./dockview-desktop-layout"; +import { CHAT_PANEL_FALLBACK_LABEL, resolveChatPanelTitle } from "./dockview-panel-content"; /** * Regression: the generic "chat" placeholder dockview panel used to fall back * to the literal "Agent" label even when the active session's agent profile * was loaded (e.g. "Opus"). The bug was a stale `isSessionTab && agentLabel` * gate inside `useChatSessionTitle` that suppressed the agent label for the - * non-session-scoped placeholder. The pure resolver below is the single place + * non-session-scoped placeholder. The pure resolver imported here is the place * the gate would have to be re-introduced, so this test pins the behavior. */ describe("resolveChatPanelTitle", () => { diff --git a/apps/web/components/task/dockview-desktop-layout.tsx b/apps/web/components/task/dockview-desktop-layout.tsx index f12df51583..1e89cd3089 100644 --- a/apps/web/components/task/dockview-desktop-layout.tsx +++ b/apps/web/components/task/dockview-desktop-layout.tsx @@ -19,27 +19,13 @@ import { setupSashDragCapToggle, } from "./dockview-layout-setup"; import { useAppStore, useAppStoreApi } from "@/components/state-provider"; -import { useFileEditors } from "@/hooks/use-file-editors"; import { useLspFileOpener } from "@/hooks/use-lsp-file-opener"; import { useEditorKeybinds } from "@/hooks/use-editor-keybinds"; import { usePlanPanelAutoOpen } from "@/hooks/use-plan-panel-auto-open"; -import { useSessionChangesCount } from "@/hooks/domains/session/use-session-changes-count"; -import { useEnvironmentSessionId } from "@/hooks/use-environment-session-id"; -import { useActiveTaskHasRepos } from "@/hooks/domains/kanban/use-active-task-has-repos"; // Panel components (rendered via portals, not directly by dockview) import { LeftHeaderActions, RightHeaderActions } from "./dockview-header-actions"; import { DockviewWatermark } from "./dockview-watermark"; -import { TaskChatPanel } from "./task-chat-panel"; -import { TaskChangesPanel } from "./task-changes-panel"; -import type { ReviewSource } from "@/hooks/domains/session/use-review-sources"; -import type { OpenDiffOptions } from "./changes-diff-target"; -import { ChangesPanel } from "./changes-panel"; -import { FilesPanel } from "./files-panel"; -import { TaskPlanPanel } from "./task-plan-panel"; -import { FileEditorPanel } from "./file-editor-panel"; -import { PassthroughToolbar } from "./passthrough-toolbar"; -import { PanelRoot, PanelBody } from "./panel-primitives"; import { ContextMenuTab } from "./tab-context-menu"; import { ChangesTab } from "./changes-tab"; import { useChangesPanelAutoFocus } from "./changes-panel-focus"; @@ -58,27 +44,18 @@ import { useCompactDockviewDefault, useDockviewUnmountCleanup, } from "./dockview-desktop-layout-hooks"; -import { TerminalPanel } from "./terminal-panel"; -import { BrowserPanel } from "./browser-panel"; -import { VscodePanel } from "./vscode-panel"; -import { CommitDetailPanel } from "./commit-detail-panel"; -import { PRDetailPanelComponent } from "@/components/github/pr-detail-panel"; +import { renderPanel } from "./dockview-panel-content"; import { PreviewController } from "./preview-controller"; -import { ReviewDialog } from "@/components/review/review-dialog"; import { BottomTerminalPanel } from "./bottom-terminal-panel"; +import { DockviewReviewDialog } from "./dockview-review-dialog"; import { useReviewDialog } from "./use-review-dialog"; import type { Repository, RepositoryScript } from "@/lib/types/http"; import type { Terminal } from "@/hooks/domains/session/use-terminals"; // Portal system -import { setPanelTitle } from "@/lib/layout/panel-portal-manager"; import { PanelPortalHost, usePortalSlot } from "@/lib/layout/panel-portal-host"; import { ENV_SCOPED_DOCKVIEW_COMPONENTS } from "@/lib/state/dockview-env-scoped-components"; -import type { AppState } from "@/lib/state/store"; -import { createDebugLogger, isDebug } from "@/lib/debug/log"; - -const debugChangesVisibility = createDebugLogger("changes:visibility"); // --------------------------------------------------------------------------- // PORTAL SLOT — generic dockview component that adopts a persistent portal @@ -201,265 +178,6 @@ const tabComponents: Record { - if (!sessionId) return null; - const session = state.taskSessions.items[sessionId]; - if (!session?.agent_profile_id) return null; - const profile = state.agentProfiles.items.find( - (p: { id: string }) => p.id === session.agent_profile_id, - ); - if (!profile) return null; - const parts = profile.label.split(" \u2022 "); - return parts[1] || parts[0] || profile.label; - }); - useEffect(() => { - setPanelTitle(panelId, resolveChatPanelTitle(agentLabel)); - }, [panelId, agentLabel]); -} - -function ChatContent({ panelId, params }: { panelId: string; params: Record }) { - const paramSessionId = params?.sessionId as string | undefined; - const storeSessionId = useAppStore((state) => state.tasks.activeSessionId); - const sessionId = paramSessionId ?? storeSessionId; - const taskId = useAppStore((state) => state.tasks.activeTaskId); - const { openFile } = useFileEditors(); - const isPassthrough = useAppStore((state) => - sessionId ? state.taskSessions.items[sessionId]?.is_passthrough === true : false, - ); - useChatSessionTitle(panelId, sessionId); - - if (isPassthrough) { - return ( - - - - - - ); - } - return ( - - ); -} - -function DiffViewerContent({ - panelId, - params, -}: { - panelId: string; - params: Record; -}) { - const selectedDiff = useDockviewStore((s) => s.selectedDiff); - const setSelectedDiff = useDockviewStore((s) => s.setSelectedDiff); - const { openFile } = useFileEditors(); - const panelKind = (params?.kind as string) ?? "all"; - const selectedPath = panelKind === "file" ? (params?.path as string) : undefined; - const sourceFilter = ((params?.source as string) || "all") as "all" | ReviewSource; - const panelSelectedDiff = panelKind === "all" ? selectedDiff : null; - const handleClosePanel = useCallback(() => { - const dockApi = useDockviewStore.getState().api; - const panel = dockApi?.getPanel(panelId); - if (dockApi && panel) dockApi.removePanel(panel); - }, [panelId]); - - return ( - setSelectedDiff(null)} - onOpenFile={openFile} - onBecameEmpty={handleClosePanel} - /> - ); -} - -function describeTaskRepositoriesForDebug(state: AppState, taskId: string | null) { - if (!taskId) return { source: "none", repositoryId: "-", repoCount: -1, repoIds: "-" }; - const task = state.kanban.tasks.find((item) => item.id === taskId); - if (task) { - return { - source: "kanban", - repositoryId: task.repositoryId ?? "-", - repoCount: task.repositories?.length ?? -1, - repoIds: task.repositories?.map((repo) => repo.repository_id).join(",") || "-", - }; - } - for (const [workflowId, snapshot] of Object.entries(state.kanbanMulti.snapshots)) { - const snapshotTask = snapshot.tasks.find((item) => item.id === taskId); - if (!snapshotTask) continue; - return { - source: `kanbanMulti:${workflowId}`, - repositoryId: snapshotTask.repositoryId ?? "-", - repoCount: snapshotTask.repositories?.length ?? -1, - repoIds: snapshotTask.repositories?.map((repo) => repo.repository_id).join(",") || "-", - }; - } - return { source: "missing", repositoryId: "-", repoCount: -1, repoIds: "-" }; -} - -function ChangesContent({ panelId }: { panelId: string }) { - const addDiffViewerPanel = useDockviewStore((s) => s.addDiffViewerPanel); - const addFileDiffPanel = useDockviewStore((s) => s.addFileDiffPanel); - const addCommitDetailPanel = useDockviewStore((s) => s.addCommitDetailPanel); - const { openFile } = useFileEditors(); - const appStore = useAppStoreApi(); - - // Dynamic title with file count — use environment-stable sessionId so the - // tab title doesn't re-fetch on same-environment session tab switches. - const activeSessionId = useEnvironmentSessionId(); - const totalCount = useSessionChangesCount(activeSessionId); - - // Repo-less tasks have no git changes ever — auto-close the panel so users - // don't see a permanently empty Changes tab. Gate on a confirmed `false`: - // `null` means the task hasn't loaded yet, and removing the panel during - // that window is unrecoverable in the same session. - const taskHasRepos = useActiveTaskHasRepos(); - useEffect(() => { - const dockApi = useDockviewStore.getState().api; - const panel = dockApi?.getPanel(panelId); - if (isDebug()) { - const state = appStore.getState(); - const activeTaskId = state.tasks.activeTaskId; - const repoDebug = describeTaskRepositoriesForDebug(state, activeTaskId); - let action = "keep"; - if (taskHasRepos === false) { - action = panel ? "remove" : "remove-missing-panel"; - } - debugChangesVisibility("auto-close decision", { - panelId, - taskHasRepos: taskHasRepos === null ? "unknown" : String(taskHasRepos), - action, - activeTaskId: activeTaskId ?? "-", - sessionId: state.tasks.activeSessionId ?? "-", - taskSource: repoDebug.source, - repositoryId: repoDebug.repositoryId, - repoCount: repoDebug.repoCount, - repoIds: repoDebug.repoIds, - livePanelIds: dockApi?.panels.map((p) => p.id).join(",") ?? "-", - }); - } - if (taskHasRepos !== false) return; - if (dockApi && panel) dockApi.removePanel(panel); - }, [taskHasRepos, panelId, appStore]); - - useEffect(() => { - const title = totalCount > 0 ? `Changes (${totalCount})` : "Changes"; - setPanelTitle(panelId, title); - }, [totalCount, panelId]); - - const handleEditFile = useCallback((path: string) => openFile(path), [openFile]); - const handleOpenDiffFile = useCallback( - (path: string, options?: OpenDiffOptions) => - addFileDiffPanel(path, { source: options?.source, repositoryName: options?.repositoryName }), - [addFileDiffPanel], - ); - const handleOpenCommitDetail = useCallback( - (sha: string, repo?: string) => addCommitDetailPanel(sha, { repo }), - [addCommitDetailPanel], - ); - const handleOpenDiffAll = useCallback(() => addDiffViewerPanel(), [addDiffViewerPanel]); - const handleOpenReview = useCallback(() => { - window.dispatchEvent(new CustomEvent("open-review-dialog")); - }, []); - - return ( - - ); -} - -function FilesContent() { - const { openFile } = useFileEditors(); - const handleOpenFile = useCallback( - (file: { path: string; name: string; content: string }) => openFile(file.path), - [openFile], - ); - return ; -} - -function PlanContent() { - const taskId = useAppStore((state) => state.tasks.activeTaskId); - return ; -} - -// --------------------------------------------------------------------------- -// renderPanel — maps component names to their portal content -// --------------------------------------------------------------------------- - -/** Resolve legacy component aliases to current names. */ -const COMPONENT_ALIASES: Record = { - "diff-files": "changes", - "all-files": "files", -}; - -function resolveComponent(component: string): string { - return COMPONENT_ALIASES[component] ?? component; -} - -function renderPanel( - panelId: string, - component: string, - params: Record, -): React.ReactNode { - const resolved = resolveComponent(component); - - switch (resolved) { - case "sidebar": - return null; - case "chat": - return ; - case "diff-viewer": - return ; - case "file-editor": - return ; - case "commit-detail": - return ; - case "changes": - return ; - case "files": - return ; - case "terminal": - return ; - case "browser": - return ; - case "vscode": - return ; - case "plan": - return ; - case "pr-detail": - return ; - default: - return
Unknown panel: {component}
; - } -} - // --------------------------------------------------------------------------- // LAYOUT RESTORATION HELPERS // --------------------------------------------------------------------------- @@ -536,32 +254,6 @@ type DockviewDesktopLayoutProps = { compact?: boolean; }; -function ReviewDialogSlot({ - sessionId, - review, -}: { - sessionId: string | null; - review: ReturnType; -}) { - if (!sessionId) { - return null; - } - - return ( - - ); -} - export const DockviewDesktopLayout = memo(function DockviewDesktopLayout({ sessionId, repository, @@ -669,7 +361,7 @@ export const DockviewDesktopLayout = memo(function DockviewDesktopLayout({ - + ); }); diff --git a/apps/web/components/task/dockview-panel-content.tsx b/apps/web/components/task/dockview-panel-content.tsx new file mode 100644 index 0000000000..4fdf597e3b --- /dev/null +++ b/apps/web/components/task/dockview-panel-content.tsx @@ -0,0 +1,276 @@ +"use client"; + +import React, { useCallback, useEffect } from "react"; +import { PRDetailPanelComponent } from "@/components/github/pr-detail-panel"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; +import { useActiveTaskHasRepos } from "@/hooks/domains/kanban/use-active-task-has-repos"; +import { useSessionChangesCount } from "@/hooks/domains/session/use-session-changes-count"; +import type { ReviewSource } from "@/hooks/domains/session/use-review-sources"; +import { useEnvironmentSessionId } from "@/hooks/use-environment-session-id"; +import { useFileEditors } from "@/hooks/use-file-editors"; +import { createDebugLogger, isDebug } from "@/lib/debug/log"; +import { setPanelTitle } from "@/lib/layout/panel-portal-manager"; +import { useDockviewStore } from "@/lib/state/dockview-store"; +import type { AppState } from "@/lib/state/store"; +import { BrowserPanel } from "./browser-panel"; +import type { OpenDiffOptions } from "./changes-diff-target"; +import { ChangesPanel } from "./changes-panel"; +import { CommitDetailPanel } from "./commit-detail-panel"; +import { FileEditorPanel } from "./file-editor-panel"; +import { FilesPanel } from "./files-panel"; +import { PanelBody, PanelRoot } from "./panel-primitives"; +import { PassthroughToolbar } from "./passthrough-toolbar"; +import { TaskChangesPanel } from "./task-changes-panel"; +import { TaskChatPanel } from "./task-chat-panel"; +import { TaskPlanPanel } from "./task-plan-panel"; +import { TerminalPanel } from "./terminal-panel"; +import { VscodePanel } from "./vscode-panel"; + +const debugChangesVisibility = createDebugLogger("changes:visibility"); + +export const CHAT_PANEL_FALLBACK_LABEL = "Agent"; + +export function resolveChatPanelTitle(agentLabel: string | null | undefined): string { + return agentLabel || CHAT_PANEL_FALLBACK_LABEL; +} + +function useChatSessionTitle(panelId: string, sessionId: string | null) { + const agentLabel = useAppStore((state) => { + if (!sessionId) return null; + const session = state.taskSessions.items[sessionId]; + if (!session?.agent_profile_id) return null; + const profile = state.agentProfiles.items.find( + (p: { id: string }) => p.id === session.agent_profile_id, + ); + if (!profile) return null; + const parts = profile.label.split(" \u2022 "); + return parts[1] || parts[0] || profile.label; + }); + useEffect(() => { + setPanelTitle(panelId, resolveChatPanelTitle(agentLabel)); + }, [panelId, agentLabel]); +} + +function ChatContent({ panelId, params }: { panelId: string; params: Record }) { + const paramSessionId = params?.sessionId as string | undefined; + const storeSessionId = useAppStore((state) => state.tasks.activeSessionId); + const sessionId = paramSessionId ?? storeSessionId; + const taskId = useAppStore((state) => state.tasks.activeTaskId); + const { openFile } = useFileEditors(); + const isPassthrough = useAppStore((state) => + sessionId ? state.taskSessions.items[sessionId]?.is_passthrough === true : false, + ); + useChatSessionTitle(panelId, sessionId); + + if (isPassthrough) { + return ( + + + + + + ); + } + return ( + + ); +} + +function DiffViewerContent({ + panelId, + params, +}: { + panelId: string; + params: Record; +}) { + const selectedDiff = useDockviewStore((s) => s.selectedDiff); + const setSelectedDiff = useDockviewStore((s) => s.setSelectedDiff); + const { openFile } = useFileEditors(); + const panelKind = (params?.kind as string) ?? "all"; + const selectedPath = panelKind === "file" ? (params?.path as string) : undefined; + const sourceFilter = ((params?.source as string) || "all") as "all" | ReviewSource; + const panelSelectedDiff = panelKind === "all" ? selectedDiff : null; + const handleClosePanel = useCallback(() => { + const dockApi = useDockviewStore.getState().api; + const panel = dockApi?.getPanel(panelId); + if (dockApi && panel) dockApi.removePanel(panel); + }, [panelId]); + + return ( + setSelectedDiff(null)} + onOpenFile={openFile} + onBecameEmpty={handleClosePanel} + /> + ); +} + +function describeTaskRepositoriesForDebug(state: AppState, taskId: string | null) { + if (!taskId) return { source: "none", repositoryId: "-", repoCount: -1, repoIds: "-" }; + const task = state.kanban.tasks.find((item) => item.id === taskId); + if (task) { + return { + source: "kanban", + repositoryId: task.repositoryId ?? "-", + repoCount: task.repositories?.length ?? -1, + repoIds: task.repositories?.map((repo) => repo.repository_id).join(",") || "-", + }; + } + for (const [workflowId, snapshot] of Object.entries(state.kanbanMulti.snapshots)) { + const snapshotTask = snapshot.tasks.find((item) => item.id === taskId); + if (!snapshotTask) continue; + return { + source: `kanbanMulti:${workflowId}`, + repositoryId: snapshotTask.repositoryId ?? "-", + repoCount: snapshotTask.repositories?.length ?? -1, + repoIds: snapshotTask.repositories?.map((repo) => repo.repository_id).join(",") || "-", + }; + } + return { source: "missing", repositoryId: "-", repoCount: -1, repoIds: "-" }; +} + +function ChangesContent({ panelId }: { panelId: string }) { + const addDiffViewerPanel = useDockviewStore((s) => s.addDiffViewerPanel); + const addFileDiffPanel = useDockviewStore((s) => s.addFileDiffPanel); + const addCommitDetailPanel = useDockviewStore((s) => s.addCommitDetailPanel); + const { openFile } = useFileEditors(); + const appStore = useAppStoreApi(); + + // Dynamic title with file count - use environment-stable sessionId so the + // tab title doesn't re-fetch on same-environment session tab switches. + const activeSessionId = useEnvironmentSessionId(); + const totalCount = useSessionChangesCount(activeSessionId); + + // Repo-less tasks have no git changes ever - auto-close the panel so users + // don't see a permanently empty Changes tab. Gate on a confirmed `false`: + // `null` means the task hasn't loaded yet, and removing the panel during + // that window is unrecoverable in the same session. + const taskHasRepos = useActiveTaskHasRepos(); + useEffect(() => { + const dockApi = useDockviewStore.getState().api; + const panel = dockApi?.getPanel(panelId); + if (isDebug()) { + const state = appStore.getState(); + const activeTaskId = state.tasks.activeTaskId; + const repoDebug = describeTaskRepositoriesForDebug(state, activeTaskId); + let action = "keep"; + if (taskHasRepos === false) { + action = panel ? "remove" : "remove-missing-panel"; + } + debugChangesVisibility("auto-close decision", { + panelId, + taskHasRepos: taskHasRepos === null ? "unknown" : String(taskHasRepos), + action, + activeTaskId: activeTaskId ?? "-", + sessionId: state.tasks.activeSessionId ?? "-", + taskSource: repoDebug.source, + repositoryId: repoDebug.repositoryId, + repoCount: repoDebug.repoCount, + repoIds: repoDebug.repoIds, + livePanelIds: dockApi?.panels.map((p) => p.id).join(",") ?? "-", + }); + } + if (taskHasRepos !== false) return; + if (dockApi && panel) dockApi.removePanel(panel); + }, [taskHasRepos, panelId, appStore]); + + useEffect(() => { + const title = totalCount > 0 ? `Changes (${totalCount})` : "Changes"; + setPanelTitle(panelId, title); + }, [totalCount, panelId]); + + const handleEditFile = useCallback((path: string) => openFile(path), [openFile]); + const handleOpenDiffFile = useCallback( + (path: string, options?: OpenDiffOptions) => + addFileDiffPanel(path, { source: options?.source, repositoryName: options?.repositoryName }), + [addFileDiffPanel], + ); + const handleOpenCommitDetail = useCallback( + (sha: string, repo?: string) => addCommitDetailPanel(sha, { repo }), + [addCommitDetailPanel], + ); + const handleOpenDiffAll = useCallback(() => addDiffViewerPanel(), [addDiffViewerPanel]); + const handleOpenReview = useCallback(() => { + window.dispatchEvent(new CustomEvent("open-review-dialog")); + }, []); + + return ( + + ); +} + +function FilesContent() { + const { openFile } = useFileEditors(); + const handleOpenFile = useCallback( + (file: { path: string; name: string; content: string }) => openFile(file.path), + [openFile], + ); + return ; +} + +function PlanContent() { + const taskId = useAppStore((state) => state.tasks.activeTaskId); + return ; +} + +const COMPONENT_ALIASES: Record = { + "diff-files": "changes", + "all-files": "files", +}; + +function resolveComponent(component: string): string { + return COMPONENT_ALIASES[component] ?? component; +} + +export function renderPanel( + panelId: string, + component: string, + params: Record, +): React.ReactNode { + const resolved = resolveComponent(component); + + switch (resolved) { + case "sidebar": + return null; + case "chat": + return ; + case "diff-viewer": + return ; + case "file-editor": + return ; + case "commit-detail": + return ; + case "changes": + return ; + case "files": + return ; + case "terminal": + return ; + case "browser": + return ; + case "vscode": + return ; + case "plan": + return ; + case "pr-detail": + return ; + default: + return
Unknown panel: {component}
; + } +} diff --git a/apps/web/components/task/dockview-review-dialog.tsx b/apps/web/components/task/dockview-review-dialog.tsx new file mode 100644 index 0000000000..95c6799652 --- /dev/null +++ b/apps/web/components/task/dockview-review-dialog.tsx @@ -0,0 +1,29 @@ +"use client"; + +import { ReviewDialog } from "@/components/review/review-dialog"; +import type { useReviewDialog } from "./use-review-dialog"; + +type DockviewReviewDialogProps = { + sessionId: string | null; + review: ReturnType; +}; + +export function DockviewReviewDialog({ sessionId, review }: DockviewReviewDialogProps) { + if (!sessionId) { + return null; + } + + return ( + + ); +} diff --git a/scripts/lint-harness-files.test.py b/scripts/lint-harness-files.test.py new file mode 100644 index 0000000000..1cfd106837 --- /dev/null +++ b/scripts/lint-harness-files.test.py @@ -0,0 +1,350 @@ +#!/usr/bin/env python3 +import importlib.util +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +SCRIPT = ROOT / ".github" / "scripts" / "lint-harness-files.py" + + +def load_linter(): + assert SCRIPT.exists(), f"{SCRIPT} does not exist" + spec = importlib.util.spec_from_file_location("lint_harness_files", SCRIPT) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class HarnessLinterTest(unittest.TestCase): + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.workdir = Path(self.tmp.name) + + def write(self, relative: str, content: str) -> Path: + path = self.workdir / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + return path + + def init_git(self) -> None: + subprocess.run(["git", "init"], cwd=self.workdir, check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + subprocess.run(["git", "add", "."], cwd=self.workdir, check=True, stdout=subprocess.DEVNULL) + + def run_cli(self, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(SCRIPT), *args], + cwd=self.workdir, + text=True, + capture_output=True, + check=False, + ) + + def test_classifies_kandev_harness_file_kinds(self) -> None: + linter = load_linter() + + cases = { + "AGENTS.md": "agent", + "nested/CLAUDE.md": "agent", + "nested/.cursorrules": "agent", + ".agents/skills/fix/SKILL.md": "skill", + ".augment/skills/fix/SKILL.md": "skill", + ".claude/skills/fix/SKILL.md": "skill", + ".cursor/skills/fix/SKILL.md": "skill", + ".opencode/skills/fix/SKILL.md": "skill", + "apps/backend/internal/office/configloader/skills/memory/SKILL.md": "skill", + ".agents/skills/fix/references/troubleshooting.md": "reference", + ".claude/skills/fix/references/troubleshooting.md": "reference", + ".cursor/skills/fix/references/troubleshooting.md": "reference", + ".opencode/skills/fix/references/troubleshooting.md": "reference", + "apps/backend/internal/office/configloader/skills/memory/REFERENCE.md": "reference", + ".agents/agents/qa.md": "role-agent", + ".opencode/agents/qa.md": "role-agent", + ".claude/agents/qa.md": "role-agent", + ".codex/agents/qa.toml": "role-agent", + ".codex/config.toml": "config", + ".claude/settings.json": "config", + ".claude/commands/pr-fixup.md": "command", + ".claude/rules/review.md": "cursor-rule", + ".cursor/rules/kandev-harness.mdc": "cursor-rule", + "README.md": "unknown", + } + + for relative, expected in cases.items(): + with self.subTest(relative=relative): + self.assertEqual(linter.classify_path(Path(relative)), expected) + + def test_line_limits_are_strict_and_disable_on_zero(self) -> None: + linter = load_linter() + path = self.write("AGENTS.md", "one\n" * 3) + + self.assertEqual(linter.lint_path(path, "agent", {"agent-line-limit": {"limit": 3}}), []) + + violations = linter.lint_path(path, "agent", {"agent-line-limit": {"limit": 2}}) + self.assertEqual([violation.rule for violation in violations], ["agent-line-limit"]) + self.assertIn("file is 3 lines (limit 2, over by 1)", violations[0].msg) + + self.assertEqual(linter.lint_path(path, "agent", {"agent-line-limit": {"limit": 0}}), []) + + def test_config_line_limit_is_wired(self) -> None: + linter = load_linter() + path = self.write(".codex/config.toml", "model = \"gpt-5\"\n" * 301) + + violations = linter.lint_path(path, "config", linter.DEFAULT_RULES_BY_KIND["config"]) + + self.assertEqual([violation.rule for violation in violations], ["config-line-limit"]) + self.assertIn("file is 301 lines (limit 300, over by 1)", violations[0].msg) + self.assertIn("agent config files are read", violations[0].msg) + + def test_description_word_limit_supports_yaml_frontmatter(self) -> None: + linter = load_linter() + path = self.write( + ".agents/skills/demo/SKILL.md", + "---\n" + "name: demo\n" + "description: one two\n" + " three four\n" + "---\n" + "\n" + "# Demo\n", + ) + + violations = linter.lint_path(path, "skill", {"description-word-limit": {"limit": 3}}) + + self.assertEqual(len(violations), 1) + self.assertEqual(violations[0].rule, "description-word-limit") + self.assertEqual(violations[0].line, 1) + self.assertIn("description is 4 words", violations[0].msg) + + def test_description_word_limit_ignores_yaml_block_scalar_markers(self) -> None: + linter = load_linter() + path = self.write( + ".agents/skills/demo/SKILL.md", + "---\n" + "name: demo\n" + "description: >\n" + " one two three four\n" + "---\n", + ) + + violations = linter.lint_path(path, "skill", {"description-word-limit": {"limit": 3}}) + + self.assertEqual(len(violations), 1) + self.assertIn("description is 4 words", violations[0].msg) + + def test_description_word_limit_is_silent_when_metadata_missing_or_disabled(self) -> None: + linter = load_linter() + missing = self.write(".agents/skills/missing/SKILL.md", "# Missing\n") + unclosed = self.write(".agents/skills/unclosed/SKILL.md", "---\ndescription: one two three\n") + + for path in (missing, unclosed): + with self.subTest(path=path): + self.assertEqual(linter.lint_path(path, "skill", {"description-word-limit": {"limit": 1}}), []) + self.assertEqual(linter.lint_path(path, "skill", {"description-word-limit": {"limit": 0}}), []) + + def test_description_word_limit_supports_codex_toml_descriptions(self) -> None: + linter = load_linter() + path = self.write( + ".codex/agents/qa.toml", + 'name = "qa"\n' + 'description = "Use when the user says \\"test it\\" before PRs."\n', + ) + + violations = linter.lint_path(path, "role-agent", {"description-word-limit": {"limit": 5}}) + + self.assertEqual(len(violations), 1) + self.assertEqual(violations[0].rule, "description-word-limit") + self.assertIn("description is 9 words", violations[0].msg) + + def test_opt_in_line_scanners_report_expected_columns(self) -> None: + linter = load_linter() + path = self.write( + "AGENTS.md", + "ok\n" + "bad \U0001f600 marker\n" + "plain \u2014 dash\n" + "\u8def see @./SKILL.md\n", + ) + config = { + "no-emoji": {}, + "no-em-dash": {}, + "no-at-dot-slash": {}, + } + + violations = linter.lint_path(path, "agent", config) + + self.assertEqual([violation.rule for violation in violations], ["no-emoji", "no-em-dash", "no-at-dot-slash"]) + self.assertIn('emoji "', violations[0].msg) + self.assertIn("column 5", violations[0].msg) + self.assertIn("column 7", violations[1].msg) + self.assertIn("column 7", violations[2].msg) + + def test_description_when_supports_yaml_frontmatter_failures(self) -> None: + linter = load_linter() + missing_frontmatter = self.write(".agents/skills/a/SKILL.md", "# Missing\n") + unclosed = self.write(".agents/skills/b/SKILL.md", "---\ndescription: Use when testing.\n") + missing_description = self.write(".agents/skills/c/SKILL.md", "---\nname: c\n---\n") + weak_description = self.write(".agents/skills/d/SKILL.md", "---\ndescription: General helper.\n---\n") + + cases = [ + (missing_frontmatter, "missing YAML frontmatter"), + (unclosed, "frontmatter opens"), + (missing_description, "has no `description:` field"), + (weak_description, "lacks a routing phrase"), + ] + + for path, expected in cases: + with self.subTest(path=path): + violations = linter.lint_path(path, "skill", {"description-when": {}}) + self.assertEqual(len(violations), 1) + self.assertIn(expected, violations[0].msg) + + def test_description_when_supports_yaml_block_scalar_descriptions(self) -> None: + linter = load_linter() + valid = self.write( + ".agents/skills/block/SKILL.md", + "---\n" + "name: block\n" + "description: >\n" + " Use when testing. Do this.\n" + "---\n", + ) + empty = self.write( + ".agents/skills/empty/SKILL.md", + "---\n" + "name: empty\n" + "description: >\n" + "---\n", + ) + + self.assertEqual(linter.lint_path(valid, "skill", {"description-when": {}}), []) + + violations = linter.lint_path(empty, "skill", {"description-when": {}}) + self.assertEqual(len(violations), 1) + self.assertIn("empty `description:` field", violations[0].msg) + + def test_description_when_supports_toml_failures(self) -> None: + linter = load_linter() + missing = self.write(".codex/agents/missing.toml", 'name = "missing"\n') + weak = self.write(".codex/agents/weak.toml", 'description = "General helper."\n') + quoted = self.write(".codex/agents/quoted.toml", 'description = "Use when the user says \\"fix it\\"."\n') + + missing_violations = linter.lint_path(missing, "role-agent", {"description-when": {}}) + weak_violations = linter.lint_path(weak, "role-agent", {"description-when": {}}) + quoted_violations = linter.lint_path(quoted, "role-agent", {"description-when": {}}) + + self.assertEqual(len(missing_violations), 1) + self.assertIn('has no `description = "..."` field', missing_violations[0].msg) + self.assertEqual(len(weak_violations), 1) + self.assertIn("lacks a routing phrase", weak_violations[0].msg) + self.assertEqual(quoted_violations, []) + + def test_toml_description_requires_parser(self) -> None: + linter = load_linter() + path = self.write(".codex/agents/qa.toml", 'description = "Use when testing."\n') + original = linter.tomllib + linter.tomllib = None + try: + with self.assertRaisesRegex(RuntimeError, "Python 3.11\\+ or the 'tomli' package"): + linter.lint_path(path, "role-agent", {"description-word-limit": {"limit": 600}}) + finally: + linter.tomllib = original + + def test_toml_description_rejects_malformed_toml(self) -> None: + linter = load_linter() + path = self.write(".codex/agents/broken.toml", 'description = "Use when broken.\n') + + with self.assertRaisesRegex(RuntimeError, "invalid TOML in harness file"): + linter.lint_path(path, "role-agent", {"description-word-limit": {"limit": 600}}) + + def test_description_when_uses_explicit_routing_phrases(self) -> None: + linter = load_linter() + weak = self.write( + ".agents/skills/trigger/SKILL.md", + "---\n" + "description: This skill dispatches a trigger event to the runtime.\n" + "---\n", + ) + strong = self.write( + ".agents/skills/trigger-on/SKILL.md", + "---\n" + "description: Trigger on task review requests. Do review setup.\n" + "---\n", + ) + + self.assertEqual(len(linter.lint_path(weak, "skill", {"description-when": {}})), 1) + self.assertEqual(linter.lint_path(strong, "skill", {"description-when": {}}), []) + + def test_codex_config_skips_description_when(self) -> None: + linter = load_linter() + config = self.write(".codex/config.toml", 'model = "gpt-5"\n') + + self.assertEqual(linter.lint_path(config, "config", {"description-when": {}}), []) + + def test_cli_all_uses_tracked_files_and_skips_untracked(self) -> None: + self.write("AGENTS.md", "ok\n") + self.write(".agents/skills/demo/SKILL.md", "line\n" * 501) + self.init_git() + self.write(".agents/skills/untracked/SKILL.md", "line\n" * 501) + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn(".agents/skills/demo/SKILL.md", result.stdout) + self.assertNotIn("untracked", result.stdout) + + def test_cli_explicit_targets_disable_rules_and_recurse_without_symlink_dirs(self) -> None: + self.write(".agents/skills/demo/SKILL.md", "line\n" * 501) + linked_target = self.write("target/AGENTS.md", "line\n" * 301) + link_dir = self.workdir / "mirror" + try: + os.symlink(linked_target.parent, link_dir) + except OSError: + link_dir = None + + disabled = self.run_cli("--disable", "skill-line-limit", ".agents/skills/demo/SKILL.md") + self.assertEqual(disabled.returncode, 0, disabled.stdout + disabled.stderr) + + recursive = self.run_cli(".") + self.assertEqual(recursive.returncode, 1) + if link_dir is not None: + self.assertNotIn("mirror/AGENTS.md", recursive.stdout) + + def test_cli_explicit_targets_can_enable_opt_in_rules(self) -> None: + self.write("AGENTS.md", "plain \u2014 dash\n") + + default = self.run_cli("AGENTS.md") + enabled = self.run_cli("--enable", "no-em-dash", "AGENTS.md") + unknown = self.run_cli("--enable", "not-a-rule", "AGENTS.md") + + self.assertEqual(default.returncode, 0, default.stdout + default.stderr) + self.assertEqual(enabled.returncode, 1) + self.assertIn("no-em-dash", enabled.stdout) + self.assertEqual(unknown.returncode, 2) + self.assertIn("unknown opt-in rule", unknown.stderr) + + def test_cli_reports_github_annotations(self) -> None: + self.write("AGENTS.md", "line\n" * 301) + env = {**os.environ, "GITHUB_ACTIONS": "true"} + result = subprocess.run( + [sys.executable, str(SCRIPT), "AGENTS.md"], + cwd=self.workdir, + env=env, + text=True, + capture_output=True, + check=False, + ) + + self.assertEqual(result.returncode, 1) + self.assertIn("::error file=AGENTS.md::", result.stdout) + + +if __name__ == "__main__": + unittest.main()