feat(seidroid-review): post the findings as one review

.github/workflows/workflow-test-self.yml

@@ -0,0 +1,32 @@
+name: Workflow tests
+# The shell and jq inside seidroid-review.yml, run against a gh stub. Nothing here
+# reaches the GitHub API, so this needs no token and no permissions.
+on:
+  pull_request:
+    paths:
+      - '.github/workflows/seidroid-review.yml'
+      - '.github/workflows/workflow-test-self.yml'
+      - 'test/seidroid-review/**'
+  push:
+    branches: [ main ]
+    paths:
+      - '.github/workflows/seidroid-review.yml'
+      - '.github/workflows/workflow-test-self.yml'
+      - 'test/seidroid-review/**'
+permissions:
+  contents: read
+jobs:
+  place-findings:
+    name: Place findings on the code
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v7
+      - name: Set up Python
+        uses: actions/setup-python@v7
+        with:
+          python-version: '3.x'
+      - name: Install the YAML reader
+        run: python3 -m pip install --quiet pyyaml
+      - name: Run the placement harness
+        run: test/seidroid-review/run.sh

test/seidroid-review/.gitignore

@@ -0,0 +1,5 @@
+# Written by run.sh: the step extracted from the workflow, the fixtures it
+# generates, and one directory of output per case.
+place.sh
+fx/gen/
+out/

test/seidroid-review/README.md

@@ -0,0 +1,36 @@
+# `Place findings on the code`
+
+Runs the placement step of `.github/workflows/seidroid-review.yml` under `bash`,
+against a `gh` stub, and checks what it posted and what it counted.
+
+```sh
+test/seidroid-review/run.sh
+```
+
+The run needs `bash`, `jq`, and `python3` with PyYAML. It exits non-zero on the
+first failed assertion count and prints a table of one row per case.
+
+## How it works
+
+`extract.py` reads the step's `run:` block and the workflow's `FINDING_MARKER`
+out of the YAML on every run, so the harness tests the file as it stands.
+
+`bin/gh` goes on `PATH` ahead of the real `gh`. It logs every call, serves
+fixture JSON through the step's own `jq`, keeps the request body the step sent,
+and decides per case whether a call succeeds. `STUB_*` variables in `run_case`
+select the fixtures and the answers.
+
+## The fixtures
+
+`fx/files*.json` are `GET /compare` responses. One JSON object each: compare
+paginates its commits, and a second page carries no `files` key, so the step
+reads one page.
+
+- `files.json` — four files, two with a patch, two without
+- `files-short.json` — the same diff with `pkg/b.go` missing, which is what a
+  truncated response looks like
+- `files-stale.json` — a different commit's diff, for the pushed-head case
+
+`fx/line-ok.tsv` and `fx/file-ok.txt` list the `path`/`side`/`line` and the
+paths the stub accepts. A finding outside them is refused, which is how the
+per-finding ladder is exercised.

test/seidroid-review/bin/gh

@@ -0,0 +1,105 @@
+#!/usr/bin/env bash
+# gh stub: logs every call, serves fixture JSON, and decides success per scenario.
+log() { printf '%s\n' "$*" >> "$STUB_LOG"; }
+
+argv=("$@")
+joined="$*"
+
+# --- read the pull request object (base.sha and changed_files) ----------------
+case "$joined" in
+  *"/pulls/$STUB_PR"|*"/pulls/$STUB_PR --jq"*)
+    log "CALL pull"
+    if [ "$STUB_BASE" = "FAIL" ]; then
+      # A failed read answers with the error object, which the step files and reads.
+      printf '%s\n' '{"message":"Not Found","status":"404"}'
+      exit 1
+    fi
+    # NONE serves a response carrying no changed_files, which is the only way the
+    # step reaches its file-count cap test.
+    if [ "${STUB_CHANGED_FILES:-}" = "NONE" ]; then
+      printf '{"number":%s,"base":{"sha":"%s"}}\n' "$STUB_PR" "$STUB_BASE"
+    else
+      printf '{"number":%s,"base":{"sha":"%s"},"changed_files":%s}\n' \
+        "$STUB_PR" "$STUB_BASE" "${STUB_CHANGED_FILES:-4}"
+    fi
+    exit 0
+    ;;
+esac
+
+# --- the diff between two commits ---------------------------------------------
+case "$joined" in
+  *"/compare/"*)
+    # The commit range the step asked for, so a test can assert which one it read.
+    for a in "${argv[@]}"; do case "$a" in *"/compare/"*) range="${a#*/compare/}" ;; esac; done
+    log "CALL compare ${range%%\?*}"
+    if [ "$STUB_FILES" = "FAIL" ]; then
+      printf '%s\n' '{"message":"Not Found","status":"404"}'
+      exit 1
+    fi
+    # One object. compare paginates its commits, and a second page carries no files
+    # key at all, so the step asks for one page and reads its files.
+    fx="$STUB_FILES"
+    case "${range%%\?*}" in
+      *"...$STUB_STALE_SHA") fx="${STUB_FILES_STALE:-$STUB_FILES}" ;;
+    esac
+    cat "$fx"
+    exit 0
+    ;;
+esac
+
+# --- create a review (the batch) ---------------------------------------------
+case "$joined" in
+  *"/reviews"*)
+    n=${#argv[@]}
+    for ((i=0;i<n;i++)); do
+      if [ "${argv[i]}" = "--input" ]; then cp "${argv[i+1]}" "$STUB_REQUEST"; fi
+    done
+    log "CALL reviews"
+    # -i puts the status line first, which is what the step reads the code from.
+    inc=no
+    for a in "${argv[@]}"; do [ "$a" = "-i" ] && inc=yes; done
+    line() { [ "$inc" = yes ] && printf 'HTTP/2.0 %s\r\nContent-Type: application/json\r\n\r\n' "$1"; }
+    case "$STUB_REVIEW" in
+      ok)         line "200 OK";                     printf '%s\n' '{"id":9001,"state":"COMMENTED"}'; exit 0 ;;
+      400)        line "400 Bad Request";            printf '%s\n' '{"message":"Problems parsing JSON"}'; exit 1 ;;
+      403)        line "403 Forbidden";              printf '%s\n' '{"message":"Resource not accessible by integration"}'; exit 1 ;;
+      413)        line "413 Payload Too Large";      printf '%s\n' '{"message":"Body is too long"}'; exit 1 ;;
+      422)        line "422 Unprocessable Entity";   printf '%s\n' '{"message":"Validation Failed","status":"422"}'; exit 1 ;;
+      422nostatus) line "422 Unprocessable Entity";  printf '%s\n' '{"message":"Validation Failed","errors":[{"resource":"PullRequestReviewComment","field":"line","code":"invalid"}]}'; exit 1 ;;
+      500)        line "500 Internal Server Error";  printf '%s\n' '{"message":"Server Error","status":"500"}'; exit 1 ;;
+      502)        line "502 Bad Gateway";            printf '%s\n' '{"message":"Bad Gateway"}'; exit 1 ;;
+      noshape)    exit 1 ;;
+    esac
+    ;;
+esac
+
+# --- create a review comment (the per-finding rungs) -------------------------
+case "$joined" in
+  *"/comments"*)
+    path="" line="" side="" subject=""
+    n=${#argv[@]}
+    for ((i=0;i<n;i++)); do
+      case "${argv[i]}" in
+        path=*)         path="${argv[i]#path=}" ;;
+        line=*)         line="${argv[i]#line=}" ;;
+        side=*)         side="${argv[i]#side=}" ;;
+        subject_type=*) subject="${argv[i]#subject_type=}" ;;
+        body=*)         body="${argv[i]#body=}" ;;
+      esac
+    done
+    printf '%s\n' "${body%%$'\n'*}" >> "$STUB_BODIES"
+    if [ "$subject" = "file" ]; then
+      log "CALL file-comment $path"
+      grep -qxF -- "$path" "$STUB_FILE_OK" && exit 0
+      printf '%s\n' '{"message":"Validation Failed","status":"422"}'
+      exit 1
+    fi
+    log "CALL line-comment $path $side $line"
+    grep -qxF -- "$path	$side	$line" "$STUB_LINE_OK" && exit 0
+    printf '%s\n' '{"message":"Validation Failed","status":"422"}'
+    exit 1
+    ;;
+esac
+
+log "CALL unhandled $joined"
+exit 1

test/seidroid-review/extract.py

@@ -0,0 +1,10 @@
+import sys, yaml
+path, step, out = sys.argv[1], sys.argv[2], sys.argv[3]
+d = yaml.safe_load(open(path, encoding="utf-8"))
+for job in d["jobs"].values():
+    for s in job.get("steps", []):
+        if s.get("name") == step:
+            open(out, "w", encoding="utf-8").write(s["run"])
+            print(d["env"]["FINDING_MARKER"])
+            sys.exit(0)
+sys.exit("step not found: " + step)

test/seidroid-review/fx/all-placeable.json

@@ -0,0 +1 @@
+[{"file": "pkg/a.go", "severity": "blocker", "detail": "First finding.\n\nA second paragraph with a\ttab.", "line": 11, "side": "RIGHT"}, {"file": "pkg/a.go", "severity": "suggestion", "detail": "Backtick `code`, a \"quote\" and a $dollar.", "line": 12, "side": "RIGHT"}, {"file": "pkg/b.go", "severity": "nit", "detail": "Third.", "line": 2, "side": "RIGHT"}, {"file": "pkg/a.go", "severity": "blocker", "detail": "On the removed line.", "line": 11, "side": "LEFT"}]

test/seidroid-review/fx/broken.json

@@ -0,0 +1 @@
+{ not json

test/seidroid-review/fx/emptyarray.json

@@ -0,0 +1 @@
+[]

test/seidroid-review/fx/file-ok.txt

@@ -0,0 +1,4 @@
+pkg/a.go
+pkg/b.go
+assets/logo.png
+pkg/huge.go

test/seidroid-review/fx/files-short.json

@@ -0,0 +1 @@
+{"files":[{"filename":"pkg/a.go","patch":"@@ -10,4 +10,6 @@\n ctx10\n-gone11\n+added11\n+added12\n ctx13"},{"filename":"assets/logo.png"},{"filename":"pkg/huge.go"}]}

test/seidroid-review/fx/files-stale.json

@@ -0,0 +1 @@
+{"files": [{"filename": "pkg/a.go", "patch": "@@ -40,3 +40,4 @@\n ctx40\n+added41\n+added42\n ctx43"}, {"filename": "pkg/b.go", "patch": "@@ -80,2 +80,3 @@\n eighty\n+eightyone\n eightytwo"}]}

test/seidroid-review/fx/files.json

@@ -0,0 +1 @@
+{"files":[{"filename":"pkg/a.go","patch":"@@ -10,4 +10,6 @@\n ctx10\n-gone11\n+added11\n+added12\n ctx13"},{"filename":"pkg/b.go","patch":"@@ -1,2 +1,3 @@\n one\n+two\n three"},{"filename":"assets/logo.png"},{"filename":"pkg/huge.go"}]}

test/seidroid-review/fx/line-ok-minus-one.tsv

@@ -0,0 +1,4 @@
+pkg/a.go	RIGHT	11
+pkg/b.go	RIGHT	2
+pkg/a.go	LEFT	11
+pkg/huge.go	RIGHT	120

test/seidroid-review/fx/line-ok.tsv

@@ -0,0 +1,5 @@
+pkg/a.go	RIGHT	11
+pkg/a.go	RIGHT	12
+pkg/b.go	RIGHT	2
+pkg/a.go	LEFT	11
+pkg/huge.go	RIGHT	120

test/seidroid-review/fx/mixed.json

@@ -0,0 +1 @@
+[{"file": "pkg/a.go", "severity": "blocker", "detail": "First finding.\n\nA second paragraph with a\ttab.", "line": 11, "side": "RIGHT"}, {"file": "pkg/a.go", "severity": "suggestion", "detail": "Backtick `code`, a \"quote\" and a $dollar.", "line": 12, "side": "RIGHT"}, {"file": "pkg/b.go", "severity": "nit", "detail": "Third.", "line": 2, "side": "RIGHT"}, {"file": "pkg/a.go", "severity": "blocker", "detail": "On the removed line.", "line": 11, "side": "LEFT"}, {"file": "pkg/a.go", "severity": "suggestion", "detail": "Outside the hunks.", "line": 999, "side": "RIGHT"}, {"file": "pkg/untouched.go", "severity": "blocker", "detail": "A file the pull request never touches.", "line": 5, "side": "RIGHT"}, {"file": "pkg/b.go", "severity": "suggestion", "detail": "No line at all."}]

test/seidroid-review/fx/nopatch.json

@@ -0,0 +1 @@
+[{"file": "pkg/huge.go", "line": 120, "side": "RIGHT", "severity": "blocker", "detail": "A line the index cannot see and the API can."}, {"file": "assets/logo.png", "line": 1, "side": "RIGHT", "severity": "nit", "detail": "A binary file has no line to take."}, {"file": "pkg/a.go", "line": 11, "side": "RIGHT", "severity": "suggestion", "detail": "An ordinary anchored finding beside them."}]

test/seidroid-review/fx/odd.json

@@ -0,0 +1 @@
+[{"file": "pkg/a.go", "line": "12", "side": "right", "severity": "nit", "detail": "A line the model wrote as a string."}, {"file": "pkg/a.go", "line": "@/etc/passwd", "side": "RIGHT", "severity": "blocker", "detail": "A line that is not a number."}, {"file": "pkg/b.go", "line": 2, "severity": "suggestion", "detail": "No side named."}, {"file": "", "line": 1, "side": "RIGHT", "severity": "nit", "detail": "No file named."}]

test/seidroid-review/fx/shift.json

@@ -0,0 +1 @@
+[{"file": "pkg/a.go", "line": 999, "side": "RIGHT", "severity": "", "detail": "An unrated finding off the hunks."}, {"file": "pkg/untouched.go", "line": 3, "side": "", "severity": "", "detail": "An unrated finding nowhere."}]