feat: add native kandev launcher (#1391)
This commit is contained in:
@@ -11,6 +11,7 @@ from typing import Any
|
||||
|
||||
MAX_INLINE_FINDINGS = 20
|
||||
MAX_COMMENT_BODY = 60000
|
||||
OMISSION_NOTICE_RESERVE = 256
|
||||
NO_FINDINGS_MARKER = "<!-- opencode-review:no-findings -->"
|
||||
DIAGNOSTIC_MARKER = "<!-- opencode-review:diagnostic -->"
|
||||
FALLBACK_MARKER = "<!-- opencode-review:fallback-findings -->"
|
||||
@@ -25,12 +26,20 @@ def required_env(name: str) -> str:
|
||||
|
||||
def gh_api(args: list[str]) -> subprocess.CompletedProcess[str]:
|
||||
gh_bin = os.environ.get("GH_BIN", "gh")
|
||||
return subprocess.run(
|
||||
[gh_bin, "api", *args],
|
||||
text=True,
|
||||
capture_output=True,
|
||||
check=False,
|
||||
)
|
||||
try:
|
||||
return subprocess.run(
|
||||
[gh_bin, "api", *args],
|
||||
text=True,
|
||||
capture_output=True,
|
||||
check=False,
|
||||
)
|
||||
except FileNotFoundError as exc:
|
||||
return subprocess.CompletedProcess(
|
||||
args=[gh_bin, "api", *args],
|
||||
returncode=127,
|
||||
stdout="",
|
||||
stderr=f"Binary not found: {exc}",
|
||||
)
|
||||
|
||||
|
||||
def list_issue_comments() -> list[dict[str, Any]]:
|
||||
@@ -102,8 +111,12 @@ def parse_findings(raw: str) -> tuple[list[Any] | None, str | None]:
|
||||
match = re.search(r"<opencode_findings>\s*(.*?)\s*</opencode_findings>", raw, re.S)
|
||||
if not match:
|
||||
return None, "No parseable findings block was found."
|
||||
payload_text = match.group(1).strip()
|
||||
fenced = re.fullmatch(r"```(?:json)?\s*(.*?)\s*```", payload_text, re.S)
|
||||
if fenced:
|
||||
payload_text = fenced.group(1).strip()
|
||||
try:
|
||||
payload = json.loads(match.group(1))
|
||||
payload = json.loads(payload_text)
|
||||
except json.JSONDecodeError as exc:
|
||||
return None, f"Findings block contained invalid JSON: {exc}"
|
||||
if not isinstance(payload, list):
|
||||
@@ -118,7 +131,7 @@ def diagnostic_body(reason: str, status: int, stdout: str, stderr: str) -> str:
|
||||
DIAGNOSTIC_MARKER,
|
||||
"**OpenCode review diagnostic**",
|
||||
"",
|
||||
"OpenCode did not produce parseable findings, so the review check failed instead of silently skipping comments.",
|
||||
"OpenCode did not produce parseable findings, so the workflow posted this diagnostic instead of silently skipping comments.",
|
||||
"",
|
||||
f"OpenCode model: `{model}`",
|
||||
f"Commit: `{short_sha}`",
|
||||
@@ -148,6 +161,7 @@ def no_findings_body() -> str:
|
||||
def fallback_body(rejected: list[dict[str, Any]], overflow: list[dict[str, Any]]) -> tuple[str, int, int]:
|
||||
model = required_env("OPENCODE_MODEL")
|
||||
short_sha = required_env("HEAD_SHA")[:12]
|
||||
body_limit = MAX_COMMENT_BODY - OMISSION_NOTICE_RESERVE
|
||||
body = "\n".join(
|
||||
[
|
||||
FALLBACK_MARKER,
|
||||
@@ -167,7 +181,7 @@ def fallback_body(rejected: list[dict[str, Any]], overflow: list[dict[str, Any]]
|
||||
if not findings:
|
||||
return
|
||||
heading = f"\n\n## {title}"
|
||||
if len(body) + len(heading) > MAX_COMMENT_BODY:
|
||||
if len(body) + len(heading) > body_limit:
|
||||
omitted += len(findings)
|
||||
return
|
||||
body += heading
|
||||
@@ -180,7 +194,7 @@ def fallback_body(rejected: list[dict[str, Any]], overflow: list[dict[str, Any]]
|
||||
str(finding["body"]).strip(),
|
||||
]
|
||||
)
|
||||
if len(body) + len(section) > MAX_COMMENT_BODY:
|
||||
if len(body) + len(section) > body_limit:
|
||||
omitted += 1
|
||||
continue
|
||||
body += section
|
||||
@@ -191,8 +205,9 @@ def fallback_body(rejected: list[dict[str, Any]], overflow: list[dict[str, Any]]
|
||||
|
||||
if omitted:
|
||||
notice = f"\n\n_Additional fallback findings omitted from this comment due to GitHub body size limits: {omitted}._"
|
||||
if len(body) + len(notice) <= MAX_COMMENT_BODY:
|
||||
body += notice
|
||||
if len(notice) > OMISSION_NOTICE_RESERVE:
|
||||
notice = notice[: OMISSION_NOTICE_RESERVE - 3].rstrip() + "..."
|
||||
body += notice
|
||||
|
||||
return body, included, omitted
|
||||
|
||||
@@ -204,7 +219,16 @@ def normalize_finding(value: Any, changed_paths: set[str]) -> dict[str, Any] | N
|
||||
line = value.get("line")
|
||||
title = value.get("title")
|
||||
body = value.get("body")
|
||||
if not isinstance(path, str) or not isinstance(line, int):
|
||||
if not isinstance(path, str):
|
||||
print(f"Skipped OpenCode finding with invalid path value: {path}")
|
||||
return None
|
||||
if isinstance(line, bool):
|
||||
print(f"Skipped OpenCode finding with invalid line value: {path}:{line}")
|
||||
return None
|
||||
if isinstance(line, float) and line.is_integer():
|
||||
line = int(line)
|
||||
if not isinstance(line, int):
|
||||
print(f"Skipped OpenCode finding with invalid line value: {path}:{line}")
|
||||
return None
|
||||
if not isinstance(title, str) or not isinstance(body, str):
|
||||
return None
|
||||
@@ -246,7 +270,16 @@ def post_inline_finding(finding: dict[str, Any]) -> tuple[bool, str]:
|
||||
return False, result.stderr.strip()
|
||||
|
||||
|
||||
def handle_diagnostic(args: argparse.Namespace, reason: str, status: int, stdout: str, stderr: str, raw: str) -> int:
|
||||
def handle_diagnostic(
|
||||
args: argparse.Namespace,
|
||||
reason: str,
|
||||
status: int,
|
||||
stdout: str,
|
||||
stderr: str,
|
||||
raw: str,
|
||||
*,
|
||||
fail_check: bool,
|
||||
) -> int:
|
||||
append_summary(
|
||||
args.summary,
|
||||
"OpenCode review diagnostic",
|
||||
@@ -261,7 +294,7 @@ def handle_diagnostic(args: argparse.Namespace, reason: str, status: int, stdout
|
||||
)
|
||||
upsert_issue_comment(DIAGNOSTIC_MARKER, diagnostic_body(reason, status, stdout, stderr))
|
||||
print(f"OpenCode did not produce parseable findings: {reason}", file=sys.stderr)
|
||||
return 1
|
||||
return 1 if fail_check else 0
|
||||
|
||||
|
||||
def post_findings(args: argparse.Namespace) -> int:
|
||||
@@ -271,11 +304,27 @@ def post_findings(args: argparse.Namespace) -> int:
|
||||
status = read_status(args.opencode_status)
|
||||
|
||||
if status != 0:
|
||||
return handle_diagnostic(args, f"OpenCode exited with status {status}.", status, stdout, stderr, raw)
|
||||
return handle_diagnostic(
|
||||
args,
|
||||
f"OpenCode exited with status {status}.",
|
||||
status,
|
||||
stdout,
|
||||
stderr,
|
||||
raw,
|
||||
fail_check=True,
|
||||
)
|
||||
|
||||
findings, error = parse_findings(raw)
|
||||
if error is not None or findings is None:
|
||||
return handle_diagnostic(args, error or "Unknown parser error.", status, stdout, stderr, raw)
|
||||
return handle_diagnostic(
|
||||
args,
|
||||
error or "Unknown parser error.",
|
||||
status,
|
||||
stdout,
|
||||
stderr,
|
||||
raw,
|
||||
fail_check=False,
|
||||
)
|
||||
|
||||
if not findings:
|
||||
posted_no_findings = upsert_issue_comment(NO_FINDINGS_MARKER, no_findings_body())
|
||||
|
||||
@@ -78,11 +78,12 @@ class OpenCodeReviewScriptTest(unittest.TestCase):
|
||||
output: str,
|
||||
inline_fail: bool = False,
|
||||
issue_comment_fail: bool = False,
|
||||
gh_bin: Path | None = None,
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
self.output_path.write_text(output, encoding="utf-8")
|
||||
env = {
|
||||
**os.environ,
|
||||
"GH_BIN": str(self.fake_gh),
|
||||
"GH_BIN": str(gh_bin or self.fake_gh),
|
||||
"GH_CALLS": str(self.calls_path),
|
||||
"GITHUB_REPOSITORY": "kdlbs/kandev",
|
||||
"PR_NUMBER": "42",
|
||||
@@ -130,10 +131,10 @@ class OpenCodeReviewScriptTest(unittest.TestCase):
|
||||
bodies.append(call[index + 1][len("body=") :])
|
||||
return bodies
|
||||
|
||||
def test_missing_findings_block_fails_and_posts_diagnostic(self) -> None:
|
||||
def test_missing_findings_block_posts_diagnostic_without_failing(self) -> None:
|
||||
result = self.run_script(output="OpenCode refused to read external_directory (/tmp/*)\n")
|
||||
|
||||
self.assertNotEqual(result.returncode, 0)
|
||||
self.assertEqual(result.returncode, 0)
|
||||
self.assertIn("did not produce parseable findings", result.stderr)
|
||||
self.assertTrue(
|
||||
any("<!-- opencode-review:diagnostic -->" in body for body in self.bodies()),
|
||||
@@ -141,14 +142,29 @@ class OpenCodeReviewScriptTest(unittest.TestCase):
|
||||
)
|
||||
self.assertIn("No parseable findings block", self.summary_path.read_text())
|
||||
|
||||
def test_invalid_findings_json_fails_and_posts_diagnostic(self) -> None:
|
||||
def test_invalid_findings_json_posts_diagnostic_without_failing(self) -> None:
|
||||
result = self.run_script(output="<opencode_findings>{}</opencode_findings>\n")
|
||||
|
||||
self.assertNotEqual(result.returncode, 0)
|
||||
self.assertEqual(result.returncode, 0)
|
||||
self.assertIn("did not produce parseable findings", result.stderr)
|
||||
self.assertTrue(any("<!-- opencode-review:diagnostic -->" in body for body in self.bodies()))
|
||||
self.assertIn("not an array", self.summary_path.read_text())
|
||||
|
||||
def test_fenced_json_findings_are_accepted(self) -> None:
|
||||
result = self.run_script(output="<opencode_findings>```json\n[]\n```</opencode_findings>\n")
|
||||
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertTrue(any("<!-- opencode-review:no-findings -->" in body for body in self.bodies()))
|
||||
|
||||
def test_missing_gh_binary_reports_gracefully(self) -> None:
|
||||
result = self.run_script(
|
||||
output="<opencode_findings>[]</opencode_findings>\n",
|
||||
gh_bin=self.workdir / "missing-gh",
|
||||
)
|
||||
|
||||
self.assertNotEqual(result.returncode, 0)
|
||||
self.assertIn("Binary not found", result.stderr)
|
||||
|
||||
def test_valid_empty_array_posts_no_findings_comment(self) -> None:
|
||||
result = self.run_script(output="<opencode_findings>[]</opencode_findings>\n")
|
||||
|
||||
@@ -174,6 +190,34 @@ class OpenCodeReviewScriptTest(unittest.TestCase):
|
||||
self.assertTrue(any("<!-- opencode-review:fallback-findings -->" in body for body in bodies))
|
||||
self.assertTrue(any("src/app.ts:99" in body for body in bodies))
|
||||
|
||||
def test_integral_float_line_values_are_normalized(self) -> None:
|
||||
result = self.run_script(
|
||||
output=textwrap.dedent(
|
||||
"""\
|
||||
<opencode_findings>
|
||||
[{"path":"src/app.ts","line":99.0,"title":"Bad line","body":"This line moved."}]
|
||||
</opencode_findings>
|
||||
"""
|
||||
),
|
||||
)
|
||||
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertTrue(any("-F" in call and "line=99" in call for call in self.read_calls()))
|
||||
|
||||
def test_invalid_path_values_are_logged(self) -> None:
|
||||
result = self.run_script(
|
||||
output=textwrap.dedent(
|
||||
"""\
|
||||
<opencode_findings>
|
||||
[{"path":42,"line":99,"title":"Bad line","body":"This line moved."}]
|
||||
</opencode_findings>
|
||||
"""
|
||||
),
|
||||
)
|
||||
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
self.assertIn("invalid path value", result.stdout)
|
||||
|
||||
def test_inline_findings_beyond_limit_are_preserved_in_fallback_comment(self) -> None:
|
||||
findings = [
|
||||
{"path": "src/app.ts", "line": index + 1, "title": f"Finding {index + 1}", "body": "body"}
|
||||
@@ -212,8 +256,12 @@ class OpenCodeReviewScriptTest(unittest.TestCase):
|
||||
self.assertEqual(result.returncode, 0, result.stderr)
|
||||
bodies = self.bodies()
|
||||
self.assertTrue(any("Additional fallback findings omitted" in body for body in bodies))
|
||||
self.assertTrue(all(len(body) <= 60000 for body in bodies))
|
||||
summary = self.summary_path.read_text()
|
||||
self.assertRegex(summary, r"Fallback findings included in comment: `1[0-9]`")
|
||||
included_match = re.search(r"Fallback findings included in comment: `([1-9][0-9]*)`", summary)
|
||||
self.assertIsNotNone(included_match, summary)
|
||||
fallback = next(body for body in bodies if "<!-- opencode-review:fallback-findings -->" in body)
|
||||
self.assertEqual(fallback.count("### src/app.ts:"), int(included_match.group(1)))
|
||||
omitted_match = re.search(r"Fallback findings omitted from comment: `([1-9][0-9]*)`", summary)
|
||||
self.assertIsNotNone(omitted_match, summary)
|
||||
|
||||
|
||||
@@ -6,9 +6,6 @@ class Kandev < Formula
|
||||
license "AGPL-3.0-only"
|
||||
version "__VERSION__"
|
||||
|
||||
# Node is required for the CLI launcher.
|
||||
depends_on "node"
|
||||
|
||||
on_macos do
|
||||
if Hardware::CPU.arm?
|
||||
url "__GITHUB_BASE__/kandev-macos-arm64.tar.gz"
|
||||
@@ -31,13 +28,10 @@ class Kandev < Formula
|
||||
|
||||
def install
|
||||
libexec.install Dir["*"]
|
||||
# cli/bin/cli.js has #!/usr/bin/env node shebang. (bin/"kandev").write_env_script
|
||||
# creates a wrapper at $HOMEBREW_PREFIX/bin/kandev that sets KANDEV_BUNDLE_DIR
|
||||
# (so the CLI launcher finds bin/ and web/ in the Cellar) and KANDEV_VERSION
|
||||
# (read by run.ts at startup so the launcher logs "release: X.Y.Z" instead of
|
||||
# "release: (env)"). Calling write_env_script on the bin directory itself would
|
||||
# name the wrapper after the target's basename (cli.js), giving the wrong name.
|
||||
(bin/"kandev").write_env_script libexec/"cli/bin/cli.js",
|
||||
# Create a stable wrapper at $HOMEBREW_PREFIX/bin/kandev that points at the
|
||||
# native launcher in the Cellar and sets the bundle/version env it uses to
|
||||
# find bin/.
|
||||
(bin/"kandev").write_env_script libexec/"bin/kandev",
|
||||
KANDEV_BUNDLE_DIR: libexec.to_s,
|
||||
KANDEV_VERSION: version.to_s
|
||||
end
|
||||
|
||||
@@ -1,17 +1,26 @@
|
||||
#!/usr/bin/env bash
|
||||
# Finalize the dist/kandev/ release layout from already-built pieces.
|
||||
# Caller must have run, in this order:
|
||||
# - scripts/release/package-cli.sh (produces dist/kandev/cli/)
|
||||
# - go build ./cmd/{kandev,agentctl} -o dist/kandev/bin/... with Vite
|
||||
# assets synced into the backend embed package before building kandev.
|
||||
# After this: dist/kandev/{bin,cli} is ready to install or tar.
|
||||
# - Vite assets synced into apps/backend/internal/webapp/embedded/generated
|
||||
# - go build ./cmd/{kandev,agentctl} -o dist/kandev/bin/...
|
||||
# After this: dist/kandev/bin is ready to install or tar.
|
||||
set -euo pipefail
|
||||
|
||||
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
|
||||
BUNDLE="$ROOT_DIR/dist/kandev"
|
||||
|
||||
if [ ! -f "$BUNDLE/cli/bin/cli.js" ]; then
|
||||
echo "Missing $BUNDLE/cli/bin/cli.js; run scripts/release/package-cli.sh first" >&2
|
||||
if [ ! -f "$BUNDLE/bin/kandev" ] && [ ! -f "$BUNDLE/bin/kandev.exe" ]; then
|
||||
echo "Missing native launcher in $BUNDLE/bin; build cmd/kandev first" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ ! -f "$BUNDLE/bin/agentctl" ] && [ ! -f "$BUNDLE/bin/agentctl.exe" ]; then
|
||||
echo "Missing agentctl in $BUNDLE/bin; build cmd/agentctl first" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
if [ ! -f "$BUNDLE/bin/agentctl-linux-amd64" ]; then
|
||||
echo "Missing linux/amd64 agentctl helper in $BUNDLE/bin; build cmd/agentctl for linux/amd64 first" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
|
||||
@@ -1,49 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
# Build the CLI and copy artifacts into dist/kandev/cli/ for inclusion in release bundles.
|
||||
#
|
||||
# The CLI bundle (dist/cli.bundle.js) is a single-file esbuild output with all Node
|
||||
# dependencies inlined. This is required for Homebrew installs where there is no
|
||||
# node_modules directory in the Cellar.
|
||||
#
|
||||
# Output layout inside dist/kandev/cli/:
|
||||
# bin/cli.js - Node entrypoint (requires #!/usr/bin/env node shebang + chmod +x)
|
||||
# dist/cli.bundle.js - Self-contained bundle with inlined deps (used by Homebrew)
|
||||
# package.json - Package metadata (used to read version at runtime)
|
||||
set -euo pipefail
|
||||
|
||||
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
|
||||
CLI_DIR="$ROOT_DIR/apps/cli"
|
||||
OUT_DIR="$ROOT_DIR/dist/kandev/cli"
|
||||
|
||||
echo "Building CLI for release bundle..."
|
||||
|
||||
# Build TypeScript
|
||||
(cd "$ROOT_DIR/apps" && pnpm --filter kandev build)
|
||||
echo " TypeScript build complete"
|
||||
|
||||
# Bundle with esbuild (inlines tree-kill and all other deps)
|
||||
(cd "$ROOT_DIR/apps" && pnpm --filter kandev bundle)
|
||||
echo " esbuild bundle complete"
|
||||
|
||||
# Copy artifacts into dist/kandev/cli/
|
||||
rm -rf "$OUT_DIR"
|
||||
mkdir -p "$OUT_DIR/bin" "$OUT_DIR/dist"
|
||||
|
||||
# Write a bin/cli.js that points at the bundled dist (for Homebrew/manual installs
|
||||
# where there is no node_modules to resolve the unbundled dist/cli.js's deps).
|
||||
# The npm package's bin/cli.js is different — it requires the unbundled dist/cli.js
|
||||
# because tree-kill etc. are installed normally via npm.
|
||||
cat > "$OUT_DIR/bin/cli.js" <<'EOF'
|
||||
#!/usr/bin/env node
|
||||
|
||||
require("../dist/cli.bundle.js");
|
||||
EOF
|
||||
chmod +x "$OUT_DIR/bin/cli.js"
|
||||
|
||||
cp "$CLI_DIR/dist/cli.bundle.js" "$OUT_DIR/dist/cli.bundle.js"
|
||||
cp "$CLI_DIR/package.json" "$OUT_DIR/package.json"
|
||||
|
||||
echo "CLI artifacts packaged at $OUT_DIR"
|
||||
echo " bin/cli.js (entrypoint, requires dist/cli.bundle.js, chmod +x)"
|
||||
echo " dist/cli.bundle.js (self-contained bundle, deps inlined)"
|
||||
echo " package.json"
|
||||
@@ -2,7 +2,7 @@
|
||||
# Generate per-platform npm runtime packages from built dist/release-assets.
|
||||
#
|
||||
# Each platform bundle (kandev-{platform}.tar.gz) is repackaged as an npm package
|
||||
# containing only bin/ and web/ (no cli/). These are the @kdlbs/runtime-* packages
|
||||
# containing the native bin/ directory. These are the @kdlbs/runtime-* packages
|
||||
# that the main kandev npm package declares as optionalDependencies.
|
||||
#
|
||||
# Usage:
|
||||
@@ -70,7 +70,7 @@ for platform in linux-x64 linux-arm64 macos-x64 macos-arm64 windows-x64; do
|
||||
rm -rf "$pkg_out"
|
||||
mkdir -p "$pkg_out"
|
||||
|
||||
# Extract only bin/ and web/ from the archive (skip cli/)
|
||||
# Extract the native runtime bundle from the archive.
|
||||
local_tmp="$pkg_out/.extract_tmp"
|
||||
mkdir -p "$local_tmp"
|
||||
tar -xzf "$archive" -C "$local_tmp"
|
||||
@@ -83,7 +83,6 @@ for platform in linux-x64 linux-arm64 macos-x64 macos-arm64 windows-x64; do
|
||||
fi
|
||||
|
||||
cp -R "$bundle_root/bin" "$pkg_out/bin"
|
||||
cp -R "$bundle_root/web" "$pkg_out/web"
|
||||
rm -rf "$local_tmp"
|
||||
|
||||
os_field="${PLATFORM_TO_OS[$platform]}"
|
||||
@@ -106,8 +105,7 @@ for platform in linux-x64 linux-arm64 macos-x64 macos-arm64 windows-x64; do
|
||||
"os": $os_field,
|
||||
"cpu": $cpu_field,
|
||||
"files": [
|
||||
"bin",
|
||||
"web"
|
||||
"bin"
|
||||
]
|
||||
}
|
||||
EOF
|
||||
|
||||
Reference in New Issue
Block a user