diff --git a/.github/workflows/weekly-code-review.yml b/.github/workflows/weekly-code-review.yml new file mode 100644 index 000000000..589385701 --- /dev/null +++ b/.github/workflows/weekly-code-review.yml @@ -0,0 +1,128 @@ +name: Weekly Code Review + +# Scheduled aggregation of slow-moving code-health signals that per-PR CI +# does not catch: unused code (knip), Rust advisories (cargo-audit), and +# TODO/FIXME backlog. The run opens (or updates) a tracking issue with the +# report and uploads the raw outputs as an artifact. +# +# Runbook: docs/WEEKLY-CODE-REVIEW.md + +on: + schedule: + # Mondays, 06:00 UTC. Early enough to land before US / EU maintainers + # start the week. Override via workflow_dispatch if needed. + - cron: "0 6 * * 1" + workflow_dispatch: + +permissions: + contents: read + issues: write + +concurrency: + group: weekly-code-review + cancel-in-progress: false + +jobs: + weekly-review: + name: Aggregate weekly signals + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: Enable Corepack (for pnpm) + run: corepack enable + + - name: Setup Node.js 24.x + uses: actions/setup-node@v4 + with: + node-version: 24.x + cache: "pnpm" + + - name: Install JS dependencies + run: pnpm install --frozen-lockfile + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@1.93.0 + + - name: Cache cargo-audit binary + id: cache-cargo-audit + uses: actions/cache@v4 + with: + path: ~/.cargo/bin/cargo-audit + key: cargo-audit-${{ runner.os }}-v1 + + - name: Install cargo-audit + if: steps.cache-cargo-audit.outputs.cache-hit != 'true' + run: cargo install cargo-audit --locked + + - name: Run weekly code-review aggregator + run: bash scripts/weekly-code-review.sh weekly-code-review-out + + - name: Upload report artifact + uses: actions/upload-artifact@v4 + with: + name: weekly-code-review-${{ github.run_id }} + path: weekly-code-review-out + retention-days: 90 + + - name: Open or update tracking issue + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const body = fs.readFileSync('weekly-code-review-out/report.md', 'utf8'); + const today = new Date().toISOString().slice(0, 10); + const title = `[Automated] Weekly code-review report — ${today}`; + const label = 'weekly-code-review'; + + // Ensure the tracking label exists (idempotent). + try { + await github.rest.issues.getLabel({ ...context.repo, name: label }); + } catch (err) { + if (err.status === 404) { + await github.rest.issues.createLabel({ + ...context.repo, + name: label, + color: 'c5def5', + description: 'Automated weekly code-review report', + }); + } else { + throw err; + } + } + + // Close previous open report(s) so only the latest stays active. + const previous = await github.paginate(github.rest.issues.listForRepo, { + ...context.repo, + state: 'open', + labels: label, + per_page: 50, + }); + for (const prev of previous) { + await github.rest.issues.createComment({ + ...context.repo, + issue_number: prev.number, + body: `Superseded by the ${today} report.`, + }); + await github.rest.issues.update({ + ...context.repo, + issue_number: prev.number, + state: 'closed', + state_reason: 'completed', + }); + } + + // Open a fresh issue for this week so maintainers triage on a + // predictable cadence instead of watching a growing thread. + const runUrl = `${process.env.GITHUB_SERVER_URL}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; + const footer = `\n---\n_Run log: ${runUrl}_`; + await github.rest.issues.create({ + ...context.repo, + title, + body: body + footer, + labels: [label], + }); diff --git a/docs/WEEKLY-CODE-REVIEW.md b/docs/WEEKLY-CODE-REVIEW.md new file mode 100644 index 000000000..017ebc1de --- /dev/null +++ b/docs/WEEKLY-CODE-REVIEW.md @@ -0,0 +1,76 @@ +# Weekly Code-Review Report + +Scheduled aggregation of slow-moving code-health signals that per-PR CI does +not catch. + +## What runs + +Workflow: [`.github/workflows/weekly-code-review.yml`](../.github/workflows/weekly-code-review.yml). +Script: [`scripts/weekly-code-review.sh`](../scripts/weekly-code-review.sh). + +The aggregator currently collects: + +| Check | Source | What it catches | +| --------------- | ----------------------------------- | ------------------------------------------------- | +| Unused code | `pnpm exec knip` (in `app/`) | Unused files, exports, dependencies, types | +| Rust advisories | `cargo audit` on core + Tauri shell | Published RustSec advisories against `Cargo.lock` | +| TODO backlog | `grep` over `src/` + `app/src/` | `TODO` / `FIXME` / `XXX` / `HACK` drift | + +Each sub-check is **best-effort**: a missing tool or transient failure is +reported inline in the Markdown, not fatal. A full lane going red never stops +the rest of the report from being produced. + +## Schedule + manual trigger + +- Cron: every Monday at **06:00 UTC** (`0 6 * * 1`). +- Manual: **Actions → Weekly Code Review → Run workflow**. +- Concurrency: one run at a time; subsequent triggers queue rather than cancel. + +## Outputs + +1. **Tracking issue** — created fresh every run, labeled `weekly-code-review`. + Previous open reports are closed with a "superseded" comment so the + maintainer triage view only shows the latest week. +2. **Artifact** — `weekly-code-review-` with: + - `report.md` — the human-readable body also used for the issue. + - `report.json` — machine-readable digest (parsed check outputs) for any + downstream tooling. + Retention: 90 days. + +## Running locally + +From the repo root: + +```bash +bash scripts/weekly-code-review.sh # writes to weekly-code-review-out/ +bash scripts/weekly-code-review.sh ./out # custom dir +``` + +Dependencies: `pnpm` for knip, `cargo-audit` for Rust advisories, `python3` +for the JSON shaping. Missing tools are skipped with a note in the report. + +## Triaging a report + +- **Unused code** — knip findings are suggestions; check the linked file + before deleting. Legitimate deletions land in a `chore(cleanup)` PR. +- **Rust advisories** — bump the affected crate (`cargo update -p ` + for a patch, or pin a workaround) and re-run `cargo audit` locally. +- **TODO backlog** — the counter is a direction signal, not an action item + on its own. Watch for a rising trend over successive weeks. + +## Disabling / overrides + +- **One-off skip** — cancel the scheduled run from the Actions tab. +- **Pause indefinitely** — comment out the `schedule:` block in + `.github/workflows/weekly-code-review.yml`. `workflow_dispatch` still works. +- **Retire** — delete the workflow + `scripts/weekly-code-review.sh` and + remove the `weekly-code-review` label. No other code references them. + +## Intentionally out of scope for the first cut + +- npm audit: Yarn v1's `audit` output is messy and noisy; revisit when the + project moves to Yarn berry or adopts `audit-ci` / GitHub's dependency + review action. +- Bundle-size diff: needs a baseline to be meaningful; separate workflow. +- AI-assisted review: CodeRabbit already runs per-PR; duplicating weekly + would be noise, not signal. diff --git a/scripts/weekly-code-review.sh b/scripts/weekly-code-review.sh new file mode 100755 index 000000000..b49e962c5 --- /dev/null +++ b/scripts/weekly-code-review.sh @@ -0,0 +1,257 @@ +#!/usr/bin/env bash +# Gather weekly code-review signals and emit a Markdown report + JSON artifact. +# +# Driven by .github/workflows/weekly-code-review.yml on a schedule; also +# runnable locally from the repo root (`bash scripts/weekly-code-review.sh`). +# The intent is to surface slow-moving drift that per-PR CI does not catch: +# unused code (knip), Rust advisories (cargo-audit), and TODO/FIXME backlog. +# +# Exit codes: +# 0 — report generated, regardless of individual check success/failure. +# Checks are best-effort: a missing tool or failing sub-check is +# recorded in the report itself, not fatal. This keeps the weekly +# schedule producing a report even when one lane is red. +# 2 — misuse (bad arguments or writable output dir not resolvable). +# +# Outputs (inside the chosen output directory, default `weekly-code-review-out`): +# report.md — human-readable Markdown summary, used for the issue body. +# report.json — machine-readable digest for downstream tooling. +# +# Usage: +# scripts/weekly-code-review.sh [output_dir] + +# NOTE: no `set -e` — every check captures its own rc and we keep going. +set -uo pipefail + +OUT_DIR="${1:-weekly-code-review-out}" +mkdir -p "$OUT_DIR" || { + echo "weekly-code-review: cannot create $OUT_DIR" >&2 + exit 2 +} +# Resolve OUT_DIR to an absolute path now — the next step `cd`s into REPO_ROOT, +# and a relative OUT_DIR would otherwise resolve against the wrong tree when +# the script is invoked from outside the repo (e.g. `bash repo/scripts/...`). +OUT_DIR="$(cd "$OUT_DIR" && pwd)" || { + echo "weekly-code-review: cannot resolve $OUT_DIR to absolute path" >&2 + exit 2 +} +MD="$OUT_DIR/report.md" +JSON="$OUT_DIR/report.json" +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +DATE_UTC="$(date -u +%Y-%m-%d)" +SHA="$(git rev-parse --short=12 HEAD 2>/dev/null || echo 'unknown')" +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +# Guard the cd — without `set -e`, a failure here would silently leave the +# script running in the caller's cwd and every per-lane path lookup below +# would mismatch (ShellCheck SC2164). +cd "$REPO_ROOT" || { + echo "weekly-code-review: cannot cd to $REPO_ROOT" >&2 + exit 2 +} + +log() { echo "[weekly-code-review] $*" >&2; } + +# ---------------------------------------------------------------- markdown --- + +: > "$MD" +{ + echo "# Weekly Code-Review Report — $DATE_UTC" + echo "" + echo "Commit: \`$SHA\` · Generated by \`scripts/weekly-code-review.sh\`" + echo "" + echo "This report aggregates slow-moving signals that per-PR CI does not" + echo "catch. Each section lists raw findings; triage is a maintainer call." + echo "" +} >> "$MD" + +# Per-check JSON fragments collected here and merged at the end. +KNIP_JSON="null" +CARGO_AUDIT_CORE_JSON="null" +CARGO_AUDIT_SHELL_JSON="null" +TODO_JSON="null" + +# ------------------------------------------------------------------ knip --- + +log "running knip" +echo "## Unused code (knip)" >> "$MD" +echo "" >> "$MD" +if ! command -v pnpm >/dev/null 2>&1; then + echo "- _pnpm not available; skipped._" >> "$MD" + KNIP_JSON='{"status":"skipped","reason":"pnpm not available"}' +elif [ ! -f app/knip.json ]; then + echo "- _knip config missing; skipped._" >> "$MD" + KNIP_JSON='{"status":"skipped","reason":"knip config missing"}' +else + # knip --reporter json prints to stdout; non-zero rc when findings exist. + (cd app && pnpm exec knip --reporter json) > "$TMP/knip.json" 2> "$TMP/knip.err" + knip_rc=$? + if [ ! -s "$TMP/knip.json" ]; then + echo "- _knip produced no output (rc=$knip_rc); check CI logs._" >> "$MD" + echo "
stderr" >> "$MD" + echo "" >> "$MD" + echo ' ```' >> "$MD" + head -c 2000 "$TMP/knip.err" >> "$MD" || true + echo "" >> "$MD" + echo ' ```' >> "$MD" + echo "
" >> "$MD" + KNIP_JSON="{\"status\":\"error\",\"rc\":$knip_rc}" + else + # Single-pass: read $TMP/knip.json once, append the markdown summary, + # and emit the JSON fragment to $TMP/knip.fragment so the shell can + # capture it without re-parsing. A parse failure writes its own + # explicit fragment so we never silently drop the raw payload. + python3 - "$TMP/knip.json" "$MD" "$TMP/knip.fragment" <<'PY' +import json, sys +path, md_path, fragment_path = sys.argv[1], sys.argv[2], sys.argv[3] +try: + data = json.load(open(path)) +except Exception as e: + with open(md_path, 'a') as f: + f.write(f"- _knip output could not be parsed: {e}._\n\n") + with open(fragment_path, 'w') as f: + f.write(json.dumps({"status": "parse_error", "error": str(e)})) + sys.exit(0) + +# knip --reporter json shape: {"issues": [ {file, files:[], exports:[], ...} ]} +# Each issue object buckets findings by category. Flatten by summing list +# lengths per category, skipping the metadata "file" key. +totals = {} +for issue in data.get("issues", []) or []: + for key, values in issue.items(): + if key == "file": + continue + if isinstance(values, list): + totals[key] = totals.get(key, 0) + len(values) +with open(md_path, 'a') as f: + if not totals: + f.write("- _No unused symbols detected._\n") + else: + for k in sorted(totals): + f.write(f"- Unused {k.replace('_',' ')}: **{totals[k]}**\n") + f.write("\n") +with open(fragment_path, 'w') as f: + f.write(json.dumps({"status": "ok", "raw": data})) +PY + if [ -s "$TMP/knip.fragment" ]; then + KNIP_JSON="$(cat "$TMP/knip.fragment")" + else + KNIP_JSON='{"status":"error","reason":"knip summary script produced no fragment"}' + fi + fi +fi + +# ----------------------------------------------------------- cargo-audit --- + +log "running cargo-audit" +echo "## Rust advisories (cargo-audit)" >> "$MD" +echo "" >> "$MD" + +run_cargo_audit() { + # `lock` is the path to a `Cargo.lock` (cargo-audit auto-detects the lock + # in the current directory; there is no `--file` flag despite older docs). + # `dir` is its containing directory — we cd there before running so the + # tool finds the right lockfile for each crate (root core vs Tauri shell). + local lock="$1" label="$2" out="$3" + local dir + dir="$(dirname "$lock")" + if [ ! -f "$lock" ]; then + echo "- $label: _Cargo.lock at \`$lock\` not found; skipped._" >> "$MD" + echo '{"status":"skipped","reason":"lock missing"}' > "$out" + return + fi + if ! command -v cargo >/dev/null 2>&1; then + echo "- $label: _cargo not available; skipped._" >> "$MD" + echo '{"status":"skipped","reason":"cargo not available"}' > "$out" + return + fi + if ! command -v cargo-audit >/dev/null 2>&1 && ! cargo audit --version >/dev/null 2>&1; then + echo "- $label: _cargo-audit not installed; skipped._" >> "$MD" + echo '{"status":"skipped","reason":"cargo-audit not installed"}' > "$out" + return + fi + (cd "$dir" && cargo audit --json) > "$TMP/audit.json" 2> "$TMP/audit.err" + local rc=$? + if [ ! -s "$TMP/audit.json" ]; then + echo "- $label: _cargo-audit produced no output (rc=$rc)._" >> "$MD" + echo "{\"status\":\"error\",\"rc\":$rc}" > "$out" + return + fi + python3 - "$TMP/audit.json" "$MD" "$label" <<'PY' +import json, sys +path, md_path, label = sys.argv[1], sys.argv[2], sys.argv[3] +data = json.load(open(path)) +vulns = data.get("vulnerabilities", {}).get("list", []) or [] +warnings = data.get("warnings", {}) or {} +warning_count = sum(len(v) for v in warnings.values()) if isinstance(warnings, dict) else 0 +with open(md_path, 'a') as f: + f.write(f"- **{label}** — vulnerabilities: **{len(vulns)}**, warnings: **{warning_count}**\n") + for v in vulns[:10]: + adv = v.get("advisory", {}) or {} + pkg = v.get("package", {}) or {} + f.write(f" - `{pkg.get('name','?')}@{pkg.get('version','?')}` — {adv.get('id','?')}: {adv.get('title','?')}\n") + if len(vulns) > 10: + f.write(f" - _…and {len(vulns)-10} more._\n") +PY + cp "$TMP/audit.json" "$out" +} + +run_cargo_audit "Cargo.lock" "openhuman core" "$TMP/audit-core.json" +run_cargo_audit "app/src-tauri/Cargo.lock" "Tauri shell" "$TMP/audit-shell.json" +echo "" >> "$MD" +CARGO_AUDIT_CORE_JSON="$(cat "$TMP/audit-core.json" 2>/dev/null || echo 'null')" +CARGO_AUDIT_SHELL_JSON="$(cat "$TMP/audit-shell.json" 2>/dev/null || echo 'null')" + +# ---------------------------------------------------------------- TODOs --- + +log "counting TODO/FIXME" +echo "## TODO / FIXME backlog" >> "$MD" +echo "" >> "$MD" +# Only count source; exclude vendored and build output. rg would be faster but +# isn't guaranteed on every runner — grep is portable. +TODO_COUNT=$(grep -RIn --binary-files=without-match \ + --include='*.rs' --include='*.ts' --include='*.tsx' \ + --exclude-dir=node_modules --exclude-dir=target --exclude-dir=dist \ + --exclude-dir=vendor --exclude-dir=.git --exclude-dir=build \ + -E '(TODO|FIXME|XXX|HACK)(:|\()' src/ app/src/ 2>/dev/null | wc -l | tr -d ' ') +TODO_COUNT="${TODO_COUNT:-0}" +echo "- Open markers (TODO/FIXME/XXX/HACK) across \`src/\` + \`app/src/\`: **$TODO_COUNT**" >> "$MD" +echo "" >> "$MD" +TODO_JSON="{\"status\":\"ok\",\"count\":$TODO_COUNT}" + +# -------------------------------------------------------------- footer --- + +{ + echo "## Runbook" + echo "" + echo "- Scope, disable switch, manual trigger, and interpretation guidance" + echo " live in [\`docs/WEEKLY-CODE-REVIEW.md\`](../docs/WEEKLY-CODE-REVIEW.md)." +} >> "$MD" + +# -------------------------------------------------------------- json --- + +python3 - "$JSON" "$KNIP_JSON" "$CARGO_AUDIT_CORE_JSON" "$CARGO_AUDIT_SHELL_JSON" "$TODO_JSON" "$DATE_UTC" "$SHA" <<'PY' +import json, sys +out_path, knip, core, shell, todo, date_utc, sha = sys.argv[1:] +def parse(s): + try: + return json.loads(s) + except Exception: + return {"status":"error","reason":"unparseable fragment"} +payload = { + "generated_at": date_utc, + "commit": sha, + "checks": { + "knip": parse(knip), + "cargo_audit_core": parse(core), + "cargo_audit_shell": parse(shell), + "todo_backlog": parse(todo), + }, +} +json.dump(payload, open(out_path, "w"), indent=2) +PY + +log "report written: $MD" +log "json written: $JSON" +exit 0