diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..5ace460 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,6 @@ +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..0b7bca8 --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,60 @@ +name: ci + +# Repo-local CI for the runner itself. No org reusable fits a bash/workflow +# tooling repo, so this thin workflow lints every script and workflow, validates +# the input schema, and runs the bash test suite. The job is named `ci` so the +# required status context is `ci / ci`, matching the org ruleset convention. + +on: + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + ci: + runs-on: ubuntu-latest + timeout-minutes: 15 + concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: shellcheck all scripts + run: | + shopt -s nullglob + files=(scripts/*.sh tests/*.sh) + echo "checking: ${files[*]}" + shellcheck --external-sources --source-path=scripts "${files[@]}" + + - name: actionlint all workflows + run: | + curl -sSfL -o actionlint.tar.gz \ + https://github.com/rhysd/actionlint/releases/download/v1.7.12/actionlint_1.7.12_linux_amd64.tar.gz + echo "8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 actionlint.tar.gz" | sha256sum -c - + tar -xzf actionlint.tar.gz actionlint + ./actionlint -color + + - name: validate input schema + fixtures + run: | + # Pinned: the same integrity bar the actionlint download above meets. + python3 -m pip install --quiet 'check-jsonschema==0.37.4' + check-jsonschema --check-metaschema review/schemas/review-input.schema.json + for f in tests/fixtures/inputs/valid-*.json; do + check-jsonschema --schemafile review/schemas/review-input.schema.json "$f" + done + for f in tests/fixtures/inputs/invalid-*.json; do + if check-jsonschema --schemafile review/schemas/review-input.schema.json "$f" 2>/dev/null; then + echo "expected $f to FAIL schema validation" >&2; exit 1 + fi + done + + - name: bash tests + run: | + ./tests/test-input-validation.sh + ./tests/test-output-validation.sh + ./tests/test-gate-integrity.sh diff --git a/.github/workflows/review-pr.yml b/.github/workflows/review-pr.yml new file mode 100644 index 0000000..5a05441 --- /dev/null +++ b/.github/workflows/review-pr.yml @@ -0,0 +1,322 @@ +name: Review PR + +# SHOC PR Review Runner — Phase 1 (spec §8, §26). +# Manually dispatched. Checks out exact PR heads read-only, runs clean +# build/test gates, generates a truthful evidence report, invokes the Fireworks +# review agent, validates its output, and publishes artifacts. +# +# This workflow NEVER writes to the product repositories or their PRs: the +# GITHUB_TOKEN carries contents:read only (listing any permission zeroes every +# unlisted scope), and product-repo access uses a GitHub App installation token +# downscoped at mint time to contents/pull-requests/metadata READ. +# +# SECURITY ARCHITECTURE — why this is two jobs: +# Running the product repos' build gates executes code authored in the PR under +# review (npm lifecycle scripts, eslint/vite/vitest configs, MSBuild targets). +# That code must never share a job with a secret, because step-level `env:` is +# not an isolation boundary: PR code can poison $GITHUB_ENV for later steps, +# read a later step's /proc environ, or overwrite the runner's own scripts. +# Therefore: +# job `gates` — executes untrusted PR code. Holds NO Fireworks key, and the +# App token is revoked before the first build command runs. +# job `review` — holds the Fireworks key. Executes NO product-repo code; it +# re-checks out this repo fresh (so tampered scripts from the +# gates job cannot follow) and consumes only text artifacts. + +on: + workflow_dispatch: + inputs: + review_type: + description: Review type + required: true + type: choice + options: [frontend, backend, paired] + frontend_pr: + description: Frontend PR number (required for frontend/paired) + required: false + type: string + backend_pr: + description: Backend PR number (required for backend/paired) + required: false + type: string + ticket: + description: SH ticket identifier or URL + required: false + type: string + review_notes: + description: "Reviewer context. Sent to the Fireworks model and kept in run artifacts for 30 days — do not paste credentials." + required: false + type: string + run_mocked_e2e: + description: Run the mocked Playwright suite (frontend/paired) + required: true + type: boolean + default: true + model: + description: Fireworks review model + required: true + type: choice + default: deepseek-v4-pro + options: [deepseek-v4-pro, kimi-k2p6] + +permissions: + contents: read + +concurrency: + group: review-pr-${{ github.event.inputs.review_type }}-${{ github.event.inputs.frontend_pr }}-${{ github.event.inputs.backend_pr }} + cancel-in-progress: false + +env: + FRONTEND_REPO: Sea-Haven-Industries/shoc-frontend-new + BACKEND_REPO: Sea-Haven-Industries/shoc-backend + COMPANION_BRANCH: dev + REVIEW_TYPE: ${{ github.event.inputs.review_type }} + TICKET: ${{ github.event.inputs.ticket }} + REVIEW_NOTES: ${{ github.event.inputs.review_notes }} + +jobs: + gates: + name: Gates (${{ github.event.inputs.review_type }}) + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - name: Checkout runner + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Configure workspace paths + # The product checkouts live OUTSIDE github.workspace so PR code is + # never a sibling of this repo's scripts. RUNNER_TEMP is only available + # as a shell variable, not in a workflow-level env block. + run: | + { + echo "WORKSPACE_DIR=$RUNNER_TEMP/workspace" + echo "ARTIFACTS_DIR=$RUNNER_TEMP/workspace/artifacts" + echo "STATE_DIR=$RUNNER_TEMP/runner-state-$(openssl rand -hex 8)" + } >> "$GITHUB_ENV" + + - name: Resolve and validate inputs + id: inputs + env: + FRONTEND_PR: ${{ github.event.inputs.frontend_pr }} + BACKEND_PR: ${{ github.event.inputs.backend_pr }} + run: ./scripts/resolve-inputs.sh + + - name: Mint read-only App token + id: app-token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ secrets.SHOC_REVIEW_APP_ID }} + private-key: ${{ secrets.SHOC_REVIEW_APP_PRIVATE_KEY }} + owner: Sea-Haven-Industries + repositories: shoc-frontend-new,shoc-backend + # Downscope at mint time so the token stays read-only even if the App + # installation is later granted broader permissions. + permission-contents: read + permission-pull-requests: read + permission-metadata: read + + - name: Resolve frontend PR head + if: steps.inputs.outputs.frontend_pr != '' + id: frontend-head + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ steps.inputs.outputs.frontend_pr }} + run: ./scripts/resolve-pr-head.sh frontend "$FRONTEND_REPO" "$PR_NUMBER" + + - name: Resolve backend PR head + if: steps.inputs.outputs.backend_pr != '' + id: backend-head + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ steps.inputs.outputs.backend_pr }} + run: ./scripts/resolve-pr-head.sh backend "$BACKEND_REPO" "$PR_NUMBER" + + - name: Checkout product repositories at exact heads + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + FRONTEND_SHA: ${{ steps.frontend-head.outputs.frontend_sha }} + BACKEND_SHA: ${{ steps.backend-head.outputs.backend_sha }} + run: ./scripts/checkout-repositories.sh + + - name: Collect review context + # Runs before any PR-authored code executes, so the collected diff and + # file contents cannot be tampered with by the build. + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + run: ./scripts/collect-context.sh + + - name: Revoke App token before running untrusted code + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + run: | + gh api -X DELETE /installation/token --silent || echo "token revoke returned non-zero (it also expires on its own)" + + # Everything below this line may execute code authored in the PR. + # No secret is present in this job from here on. + + - name: Set up .NET + if: inputs.review_type == 'backend' || inputs.review_type == 'paired' + uses: actions/setup-dotnet@a98b56852c35b8e3190ac28c8c2271da59106c68 # v6.0.0 + with: + dotnet-version: 8.0.x + + - name: Backend gates + if: inputs.review_type == 'backend' || inputs.review_type == 'paired' + continue-on-error: true + run: ./scripts/run-backend-gates.sh + + - name: Set up Node + if: inputs.review_type == 'frontend' || inputs.review_type == 'paired' + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: "24" + + - name: Frontend gates + if: inputs.review_type == 'frontend' || inputs.review_type == 'paired' + continue-on-error: true + env: + RUN_MOCKED_E2E: ${{ inputs.run_mocked_e2e }} + run: ./scripts/run-frontend-gates.sh + + - name: Stop stray background processes + if: always() + run: | + # PR-authored scripts can background processes that would otherwise + # keep running and mutate files after the gates finish. + pkill -u "$(id -u)" -f 'node|dotnet|vite|playwright' 2>/dev/null || true + sleep 2 + + - name: Stage gate results for the review job + if: always() + run: | + cp "$STATE_DIR/gate-status.tsv" "$ARTIFACTS_DIR/gate-status.tsv" 2>/dev/null || true + ls -la "$ARTIFACTS_DIR" + + - name: Upload gates context + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: gates-context-${{ github.run_id }} + path: ${{ runner.temp }}/workspace/artifacts/ + retention-days: 1 + if-no-files-found: warn + + review: + name: Review + needs: gates + if: always() && needs.gates.result != 'cancelled' + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout runner + # Fresh checkout: scripts tampered with in the gates job cannot follow. + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Configure workspace paths + # The product checkouts live OUTSIDE github.workspace so PR code is + # never a sibling of this repo's scripts. RUNNER_TEMP is only available + # as a shell variable, not in a workflow-level env block. + run: | + { + echo "WORKSPACE_DIR=$RUNNER_TEMP/workspace" + echo "ARTIFACTS_DIR=$RUNNER_TEMP/workspace/artifacts" + echo "STATE_DIR=$RUNNER_TEMP/runner-state-$(openssl rand -hex 8)" + } >> "$GITHUB_ENV" + + - name: Download gates context + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: gates-context-${{ github.run_id }} + path: ${{ runner.temp }}/workspace/artifacts + + - name: Point the gate table at the downloaded results + run: | + mkdir -p "$STATE_DIR" + cp "$ARTIFACTS_DIR/gate-status.tsv" "$STATE_DIR/gate-status.tsv" 2>/dev/null || true + + - name: Generate evidence report + run: ./scripts/generate-evidence.sh + + - name: Run review agent + id: agent + continue-on-error: true + env: + FIREWORKS_API_KEY: ${{ secrets.FIREWORKS_API_KEY }} + MODEL: ${{ inputs.model }} + run: ./scripts/run-review-agent.sh + + - name: Artifact redaction check + id: redact + # Runs even when earlier steps failed so nothing is uploaded unscanned. + if: always() + env: + FIREWORKS_API_KEY: ${{ secrets.FIREWORKS_API_KEY }} + run: ./scripts/redact-check.sh + + - name: Job summary + if: always() && steps.redact.outcome == 'success' + run: | + { + echo "## SHOC PR Review — ${REVIEW_TYPE}" + echo "" + echo "### Gate status (machine-recorded, authoritative)" + echo "" + echo "| Gate | Status |" + echo "| --- | --- |" + if [ -f "$STATE_DIR/gate-status.tsv" ]; then + awk -F'\t' '{printf "| %s | %s |\n", $1, $2}' "$STATE_DIR/gate-status.tsv" + fi + echo "" + if [ -f "$ARTIFACTS_DIR/review.md" ] && [ "${{ steps.agent.outcome }}" = "success" ]; then + echo "### Review (model-generated, copy into GitHub manually)" + echo "" + echo "The block below is model output influenced by PR content. The" + echo "gate table above is the authoritative record of what ran." + echo "" + echo '```markdown' + cat "$ARTIFACTS_DIR/review.md" + echo '```' + else + echo "### No valid review produced" + echo "" + echo "Agent step outcome: ${{ steps.agent.outcome }} — see validation-errors.txt in the artifacts." + fi + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload review + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() && steps.redact.outcome == 'success' + with: + name: review-${{ github.run_id }} + path: | + ${{ runner.temp }}/workspace/artifacts/review.md + ${{ runner.temp }}/workspace/artifacts/review-evidence.md + ${{ runner.temp }}/workspace/artifacts/gate-status.tsv + ${{ runner.temp }}/workspace/artifacts/validation-errors.txt + ${{ runner.temp }}/workspace/artifacts/logs/ + retention-days: 30 + if-no-files-found: warn + + - name: Upload prompt and diff material + # Contains full private-repo source; kept briefly for debugging only. + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() && steps.redact.outcome == 'success' + with: + name: review-context-${{ github.run_id }} + path: | + ${{ runner.temp }}/workspace/artifacts/agent-prompt*.txt + ${{ runner.temp }}/workspace/artifacts/agent-raw-response* + ${{ runner.temp }}/workspace/artifacts/prompt-*.txt + ${{ runner.temp }}/workspace/artifacts/*.diff + retention-days: 2 + if-no-files-found: warn + + - name: Fail run if agent or validation failed + if: steps.agent.outcome != 'success' + run: | + echo "Review agent or output validation failed — see artifacts." >&2 + exit 1 diff --git a/README.md b/README.md index c76e608..0bd4bb4 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,169 @@ # shoc-pr-review-runner Private, cloud-hosted PR review runner for the SHOC project -(`shoc-frontend-new` + `shoc-backend`). +(`shoc-frontend-new` + `shoc-backend`). A manually-dispatched GitHub Actions +workflow checks out the exact PR head(s) in a clean environment, runs the +repositories' real build/test gates, generates a truthful evidence report, +invokes a Fireworks-hosted review model with the SHOC review skill, validates +the output deterministically, and publishes the review + evidence as run +artifacts for a human to copy into GitHub. -Implementation lands in the phase 1 pull request. +**The runner never writes to the product repositories or their PRs.** The +workflow token has `contents: read` only, product-repo access uses a read-only +GitHub App installation token, and no write-scoped credential exists in this +repository. + +## Running a review + +1. Actions → **Review PR** → *Run workflow*. +2. Choose `review_type` (`frontend` | `backend` | `paired`) and enter the PR + number(s) or URL(s). Optional: ticket, reviewer notes, model. +3. When the run finishes, the review appears in the job summary and in the + `review-` artifact together with `review-evidence.md` and all logs. +4. Copy `review.md` into the GitHub PR manually. The runner never posts it. + +Single-repo reviews check out the companion repository at its `dev` head for +contract context; it is not built or reviewed. + +## Phase status + +Phase 1 (current): input validation, exact-head checkout, clean build/test +gates, mocked Playwright, evidence report, agent invocation, output validation, +artifacts. **Not yet implemented** (spec Phases 2–4): disposable SQL Server + +migrations, backend/frontend startup + health gates, live Playwright against a +real backend, stacked/paired PR intelligence. The evidence report marks all of +these NOT_RUN — reviews cannot claim them. + +## Layout + +| Path | Purpose | +| --- | --- | +| `skills/pr-review/` | Coordinating skill + frontend/backend checklists + output contract | +| `.github/workflows/review-pr.yml` | The review workflow (workflow_dispatch) | +| `.github/workflows/ci.yaml` | Repo CI: shellcheck, actionlint, schema check, bash tests | +| `scripts/` | Orchestration scripts (see headers in each) | +| `review/runner-config.yml` | Source-of-truth config record (mirrored by the workflow env) | +| `review/schemas/` | Input contract schema | +| `templates/` | Evidence / request / failure-summary templates | +| `tests/` | Bash test suites + fixtures (run in CI) | + +## Required secrets + +| Secret | Purpose | +| --- | --- | +| `SHOC_REVIEW_APP_ID` | GitHub App ID (read-only app, see below) | +| `SHOC_REVIEW_APP_PRIVATE_KEY` | The App's private key (PEM) | +| `FIREWORKS_API_KEY` | Fireworks inference API key | + +`run-review-agent.sh` fails fast with a clear error when the Fireworks key is +missing or rejected. + +## GitHub App + +The App must have **exactly** these permissions and nothing else, installed on +**only** `shoc-frontend-new` and `shoc-backend`: + +- Repository permissions: Contents **Read-only**, Pull requests **Read-only**, + Metadata **Read-only**. + +Verify after installing (and re-verify when the App changes): + +```sh +APP_INSTALLS=$(gh api /orgs/Sea-Haven-Industries/installations --jq \ + '.installations[] | select(.app_slug=="")') +echo "$APP_INSTALLS" | jq '{permissions, repository_selection}' +# permissions must be exactly {contents: "read", pull_requests: "read", metadata: "read"} +gh api "/user/installations/$(echo "$APP_INSTALLS" | jq -r .id)/repositories" \ + --jq '.repositories[].full_name' +# must list exactly the two product repos +``` + +## Security architecture + +Reviewing a PR means building it, and building it means executing code the PR +author wrote (npm lifecycle scripts, eslint/vite/vitest configs, MSBuild +targets). The workflow is therefore split into two jobs: + +| Job | Executes PR code | Secrets present | +| --- | --- | --- | +| `gates` | yes | App token, revoked before the first build command runs | +| `review` | no | Fireworks API key only | + +Step-level `env:` is not an isolation boundary inside a job — PR code can +append to `$GITHUB_ENV` to alter later steps, read a later step's process +environment, or overwrite the runner's own scripts. The job split is what makes +those attacks worthless: by the time any PR code runs, the gates job holds no +usable credential, and the review job re-checks out this repository fresh so +tampered scripts cannot follow it. + +Further controls: + +- Product repos are checked out under `$RUNNER_TEMP`, never beside this repo's + `scripts/`. +- The App token is downscoped **at mint time** (`permission-contents: read` and + friends), so it stays read-only even if the App installation is later granted + broader permissions. +- Fork PR heads are refused outright. +- Gate results are recorded once per key outside the workspace, and every + decision path calls `assert_gate_table_intact` first: a duplicate key means + something other than the runner wrote the table, and the run fails closed + rather than trusting a forged `PASS`. +- Changed-file contents are read from git objects, never the filesystem, so a + symlink committed in a PR cannot pull host files into the prompt. +- The review prompt fences untrusted material with a per-run nonce, and + `validate-review-output.sh` re-checks every claim against the recorded gate + table rather than trusting the model. + +**Residual risk, stated plainly:** someone with push access to a product repo +can make their own PR's gates report success by having the build fake it. The +integrity check turns the obvious forms of that into a hard failure, but a CI +system that builds untrusted code cannot fully certify its own results. The +review is a reviewing aid, not an authority — a human still reads the diff. + +## Data flow to third parties + +The PR diff and the contents of changed files are sent to **Fireworks AI** +(`api.fireworks.ai`) as the review prompt. This is private SHOC source leaving +the Sea Haven boundary to an external inference provider. Confirm the Fireworks +account has training and retention disabled before reviewing anything sensitive. + +Artifacts are split so raw source is not retained as long as the review: + +| Artifact | Contents | Retention | +| --- | --- | --- | +| `review-` | review, evidence report, gate table, gate logs | 30 days | +| `review-context-` | prompt, model responses, raw diffs | 2 days | + +Anyone with read access to this repository can read those artifacts. Keep this +repository's read audience no broader than both product repositories'. + +## Artifact hygiene + +`scripts/redact-check.sh` scans every staged artifact for the run's secret +values before upload and blocks the upload on any hit. If sensitive content is +ever discovered in a published artifact, delete it immediately: + +```sh +gh api repos/Sea-Haven-Industries/shoc-pr-review-runner/actions/artifacts \ + --jq '.artifacts[] | {id, name, created_at}' +gh api -X DELETE \ + repos/Sea-Haven-Industries/shoc-pr-review-runner/actions/artifacts/ +``` + +Artifact retention is 30 days. + +## Provisioning notes + +- Repo settings: `allow_auto_merge` + `delete_branch_on_merge` enabled; org + Code Security Configuration "Sea Haven Standard" attached. +- **CodeQL exemption:** this repository contains only shell, YAML, Markdown, + and JSON — no CodeQL-supported language — so CodeQL default setup is not + enabled. Revisit if a supported language is ever added. +- Dependabot: `github-actions` ecosystem, weekly. + +## Review instructions live here, not in the product repos + +The frontend and backend review checklists are deliberately **not** committed +to `shoc-frontend-new` or `shoc-backend` (spec §27.1). They were ported from +the reviewers' local `.cursor/commands/pr-review.md` files on 2026-07-29; this +repository is now their source of truth. diff --git a/docs/phase-2-todo.md b/docs/phase-2-todo.md new file mode 100644 index 0000000..4b9d3f2 --- /dev/null +++ b/docs/phase-2-todo.md @@ -0,0 +1,147 @@ +# Phase 2 TODO — Runtime Environment (disposable DB, startup, API checks) + +Scope (spec Phase 2): disposable database, migration validation, backend +startup + health gate, frontend startup, shared environment variables, API +runtime checks, process/log management. Everything below turns an existing +`NOT_RUN` line in `review-evidence.md` into a real gate recorded through +`scripts/lib.sh` (`record_gate`/`run_gate`). Facts about the product repos were +verified 2026-07-29; trust them over the original spec draft (which wrongly +said PostgreSQL — the backend is EF Core 8.0.8 + SQL Server). + +## Ordered work plan + +### 1. Workflow: SQL Server service container +- Add a `services: mssql` block to the `review` job in + `.github/workflows/review-pr.yml`: `mcr.microsoft.com/mssql/server:2022-latest`, + port `1433:1433`, `ACCEPT_EULA=Y`, `MSSQL_SA_PASSWORD` (see open questions), + container health-cmd so the job waits for readiness. No compose file exists in + shoc-backend; the service container is the whole database story. +- Install `sqlcmd` on the runner (`mssql-tools18` apt package) in a step gated + on backend/paired, before provisioning. +- Generate per-run runtime secrets in an early step: `JWT_SECRET` + (`openssl rand -hex 32` — must be ≥32 chars or login 500s) and the DB + password if per-run. Add both to `redact-check.sh`'s scan set. + +### 2. New scripts (all source `lib.sh`, all gated on backend/paired unless noted) +- `scripts/proc.sh` — shared process-management helpers sourced next to + `lib.sh`: `start_bg ` (nohup, PID to + `$ARTIFACTS_DIR/pids/.pid`, log to `$LOG_DIR`), `stop_bg`, + `stop_all_bg` (kill + wait, idempotent, never fails the caller). +- `scripts/provision-database.sh` — wait for SQL Server, `CREATE DATABASE + ShocReview` via sqlcmd; gate `backend.db_provision`. +- `scripts/run-migration-gates.sh` — migration validation (see §3); gates + `backend.migration_list`, `backend.migration_script`, `backend.migration_apply`. +- `scripts/seed-admin-user.sh` — direct-SQL identity seed (see §4); gate + `backend.seed_admin`. +- `scripts/start-backend.sh` — start the API via `proc.sh`, poll health, run + the DB-touching assertion (see §5); gates `backend.startup`, `backend.health`. +- `scripts/run-api-checks.sh` — authenticated runtime scenarios (see §6); + gate `backend.api_runtime` (plus per-scenario detail in the gate table). +- `scripts/start-frontend.sh` — frontend runtime startup (see §7); gates + `frontend.dev_startup`, `frontend.preview_build`, `frontend.preview_startup`. + Gated on frontend/paired; runtime API checks require paired (else NOT_RUN + with reason "no live backend in frontend-only review"). +- `scripts/stop-runtime.sh` — calls `stop_all_bg`; wired as an `if: always()` + workflow step so backend/frontend processes are stopped even after failed + stages (spec requirement), before evidence generation and artifact upload. + +### 3. Migration validation detail +- 43 migrations live in `Data.SeaHavenIndustries/Migrations/`; there is NO + migrate-on-startup and NO `IDesignTimeDbContextFactory`. +- `dotnet tool restore` must run from `Api.SeaHavenIndustries/` (the tool + manifest — dotnet-ef 8.0.8, rollForward false — is at + `Api.SeaHavenIndustries/.config/dotnet-tools.json`). +- Every `dotnet ef` call needs `--project Data.SeaHavenIndustries/... --startup-project + Api.SeaHavenIndustries/...`. +- Gates: `migrations list` (enumerates cleanly), `migrations script + --idempotent -o $ARTIFACTS_DIR/migrations.sql` (SQL generation as artifact), + then apply. Prefer `database update`; the repo's own deploy uses + `ef migrations bundle --self-contained -r linux-x64` (see its + `scripts/package-elastic-beanstalk.sh`) — bundle is the fidelity option if + `database update` misbehaves. + +### 4. Seeding strategy (first-admin bootstrap gap) +- No register endpoint; `UserController.AddUser` is `[Authorize]`; API user + creation emails passwords via SendGrid; the Program.cs role-seeding block is + fully commented out (Program.cs:199-253). So: seed by direct SQL. +- `seed-admin-user.sh` runs a `templates/seed-admin.sql` via sqlcmd inserting + `AspNetRoles` ("Admin" — the only role enforced in `[Authorize]` attributes), + `AspNetUsers` (fixed reviewer account, precomputed ASP.NET Identity v3 + PBKDF2 password hash), `AspNetUserRoles`. +- The known password + hash pair is committed (disposable localhost-only DB; + document as non-sensitive). Run after migration apply, before startup. + +### 5. Backend startup + health +- Start `dotnet run --project Api.SeaHavenIndustries/... --no-build -c Release` + (or run the built DLL) via `proc.sh` with the shared env (§8). +- NO anonymous /health endpoint exists (the only health-ish route is + Admin-authorized). Health probe = `GET /swagger/v1/swagger.json` with retry + budget; requires `ASPNETCORE_ENVIRONMENT=Development` (anything else disables + Swagger AND enables HTTPS redirect). +- Swagger 200 ≠ DB configured: the committed appsettings placeholder + `"${CONNECTION_STRING}"` lets the app start and fail per-request. So the + health gate also asserts DB wiring: `POST /api/Authentication/login` with bad + creds must return 401 — a 500 means the connection string didn't take. +- Expect log noise: 4 hosted services start at boot; the vendor-document scan + worker polls the DB every 30s. Capture full stdout/stderr to + `logs/backend-runtime.log`; evidence links it. + +### 6. API runtime checks +- With the seeded admin: login → 200 + JWT; call one Admin-authorized + endpoint with the token → 200; call it unauthenticated → 401. +- Record known limitation: ClamAV__Host is empty, so vendor uploads return + 423 — assert-and-record as limitation, not failure. + +### 7. Frontend runtime +- Dev startup gate: `npm run dev` with `VITE_API_TARGET=http://127.0.0.1:5141` + (the dev proxy target env var), probe `http://127.0.0.1:3000/` for 200. +- Production preview: `vite preview` (4173) has NO /api proxy, so a preview + against the local backend needs a second build with absolute + `VITE_API_URL=http://127.0.0.1:5141/api` baked at BUILD time (must end in + `/api` or the build-time contract guard throws; the Phase-1 gate build uses + relative `/api` and cannot be reused). Then probe 4173. +- Playwright's own dev server on 4173 is untouched (mocked suite unchanged). + +### 8. Shared environment variables (one place: workflow env + runner-config) +- `ConnectionStrings__DefaultConnection` = `Server=127.0.0.1,1433;Database=ShocReview;User Id=sa;Password=...;TrustServerCertificate=True` +- `JWT__Secret` = per-run ≥32 chars (the committed `"${JWT_SECRET}"` + placeholder passes the null check but breaks login — never rely on it) +- `ASPNETCORE_ENVIRONMENT=Development`, `ASPNETCORE_URLS=http://127.0.0.1:5141` +- `WorkOrderIngest__Enabled=false`, `Sync__Enabled=false`, + `WorkOrderReconciliation__Enabled=false`, `ClamAV__Host=` (empty) + +### 9. `review/runner-config.yml` additions +- `database:` extend with db name, sa-password sourcing, sqlcmd tooling. +- `backend:` add `runtime_env` block (§8 values), health retry budget, + seed account name, migration project paths. +- `frontend:` add `preview_port: 4173`, `dev_api_target`, preview build env. +- Keep the workflow-env mirror rule (CI cross-checks the pair). + +### 10. Evidence report (`generate-evidence.sh`) +- Backend Gates: Migration list/script/apply, Startup, Health endpoint, API + runtime scenarios switch from hardcoded NOT_RUN to `$(be_gate ...)`; add DB + provision + admin seed lines. +- Frontend Gates: Development startup and Production preview switch to + `$(fe_gate ...)`. +- Runtime Limitations rewritten: enumerate disabled integrations (ingest, + sync, reconciliation, ClamAV → uploads 423), note JWT secret and DB are + runner-provided, keep "mocked Playwright ≠ live coverage". +- Tests: new fixtures in `tests/fixtures/artifacts/` covering runtime-gate + PASS/FAIL rows; extend CI bash tests for `proc.sh` start/stop semantics. + +## Out of scope (Phase 3+) +- Live Playwright against the running stack, affected-route walking, console/ + failed-request capture (Phase 3). Stacked/paired PR intelligence (Phase 4). +- Fixing shoc-backend itself (health endpoint, seeding block) — record gaps. + +## Open questions +1. `MSSQL_SA_PASSWORD`: fixed throwaway (service env can't consume step + outputs) vs repo secret? Leaning fixed + documented non-sensitive. +2. sqlcmd via apt `mssql-tools18` vs `docker exec` into the service container? +3. Migration apply: `dotnet ef database update` vs the repo's own bundle path? +4. Is the dev-server startup gate worth its runtime once preview startup + exists, or is preview + dev-proxy config check enough? +5. Which Admin endpoint is the canonical authenticated smoke check (stable, + read-only, no side effects/emails)? +6. Should `backend.health` failing hard-block the frontend preview gates in + paired runs (BLOCKED) or let them probe independently? diff --git a/review/runner-config.yml b/review/runner-config.yml new file mode 100644 index 0000000..e0f39d2 --- /dev/null +++ b/review/runner-config.yml @@ -0,0 +1,57 @@ +# SHOC PR Review Runner configuration record (spec §23). +# +# This file is the human-readable source of truth for the values the workflow +# and scripts use. The workflow's env block mirrors these values; if you change +# one here, change it there in the same PR (CI cross-checks the pair). +# Values were verified against the real repositories on 2026-07-29. + +repositories: + frontend: + name: Sea-Haven-Industries/shoc-frontend-new + path: workspace/frontend + default_branch: dev # dev is the live integration branch + node_version: "24" # repo has no .nvmrc; engines >=22.22.1, CI uses 24 + install_command: npm ci # HUSKY=0 (prepare: husky runs on install) + lint_command: npm run lint + build_command: npm run build # tsc -b && vite build (no separate typecheck script) + test_command: npm test # vitest run + e2e_mocked_command: npm run test:e2e # Playwright, fully page.route-mocked + build_env: + VITE_API_URL: /api # must end in /api (build-time contract guard); + # committed .env.production would otherwise bake + # the deployed dev API URL into the bundle + port: 3000 # dev server; Playwright drives its own on 4173 + + backend: + name: Sea-Haven-Industries/shoc-backend + path: workspace/backend + default_branch: dev + dotnet_version: 8.0.x # no global.json; matches repo CI + solution: SeaHavenIndustries.sln + startup_project: Api.SeaHavenIndustries/Api.SeaHavenIndustries.csproj # Phase 2 + restore_command: dotnet restore SeaHavenIndustries.sln + build_command: dotnet build SeaHavenIndustries.sln --configuration Release --no-restore + test_command: dotnet test SeaHavenIndustries.sln --configuration Release --no-build + port: 5141 # Phase 2: launchSettings http profile + health_path: /swagger/v1/swagger.json # Phase 2: no anonymous /health exists; + # requires ASPNETCORE_ENVIRONMENT=Development + +database: # Phase 2 (not provisioned by Phase 1) + engine: sqlserver # spec draft said postgres; the backend is EF Core + image: mcr.microsoft.com/mssql/server:2022-latest # + SqlServer — corrected + port: 1433 + +agent: + provider: fireworks + base_url: https://api.fireworks.ai/inference/v1 + default_model: deepseek-v4-pro + allowed_models: [deepseek-v4-pro, kimi-k2p6] # enforced by the workflow choice input + diff_max_bytes: 200000 + files_max_bytes: 120000 + file_max_bytes: 65536 + +review: + artifact_retention_days: 30 + companion_branch: dev # single-repo reviews check out the companion here + require_live_browser_for_frontend_approval: false # Phase 3 flips this + allow_mocked_suite_as_live_evidence: false diff --git a/review/schemas/review-input.schema.json b/review/schemas/review-input.schema.json new file mode 100644 index 0000000..36b97f6 --- /dev/null +++ b/review/schemas/review-input.schema.json @@ -0,0 +1,53 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "SHOC PR review request", + "description": "Contract for the review-pr workflow_dispatch inputs (spec §8). The workflow enforces this via resolve-inputs.sh; the schema documents the contract and validates test fixtures in CI.", + "type": "object", + "additionalProperties": false, + "required": ["review_type"], + "properties": { + "review_type": { + "enum": ["frontend", "backend", "paired"] + }, + "frontend_pr": { + "type": "string", + "pattern": "^([0-9]+|https://github\\.com/Sea-Haven-Industries/shoc-frontend-new/pull/[0-9]+(/.*)?)$", + "description": "Required when review_type is frontend or paired" + }, + "backend_pr": { + "type": "string", + "pattern": "^([0-9]+|https://github\\.com/Sea-Haven-Industries/shoc-backend/pull/[0-9]+(/.*)?)$", + "description": "Required when review_type is backend or paired" + }, + "ticket": { + "type": "string", + "maxLength": 200 + }, + "review_notes": { + "type": "string", + "maxLength": 4000 + }, + "run_mocked_e2e": { + "type": "boolean", + "default": true + }, + "model": { + "enum": ["deepseek-v4-pro", "kimi-k2p6"], + "default": "deepseek-v4-pro" + } + }, + "allOf": [ + { + "if": { "properties": { "review_type": { "const": "frontend" } } }, + "then": { "required": ["review_type", "frontend_pr"] } + }, + { + "if": { "properties": { "review_type": { "const": "backend" } } }, + "then": { "required": ["review_type", "backend_pr"] } + }, + { + "if": { "properties": { "review_type": { "const": "paired" } } }, + "then": { "required": ["review_type", "frontend_pr", "backend_pr"] } + } + ] +} diff --git a/scripts/checkout-repositories.sh b/scripts/checkout-repositories.sh new file mode 100755 index 0000000..2eeaa6a --- /dev/null +++ b/scripts/checkout-repositories.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash +# Check out the product repositories at exact SHAs (spec §10). +# +# Repos under review are checked out at their PR head SHA. The companion repo of +# a single-repo review is checked out at its default branch head for contract +# context (spec §10.2) and recorded as such. +# +# Auth: the read-only App installation token is injected ONLY as a host-scoped +# Basic http.extraHeader via GIT_CONFIG_* environment variables for the fetch +# commands. It is never placed in a URL, never Bearer, and never written to +# .git/config, so nothing credential-bearing persists after the run. +# +# Usage: checkout-repositories.sh +# Reads: GH_TOKEN, REVIEW_TYPE, FRONTEND_REPO, BACKEND_REPO, +# FRONTEND_SHA / BACKEND_SHA (empty when that side has no PR), +# COMPANION_BRANCH (default: dev) + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +require_env GH_TOKEN REVIEW_TYPE FRONTEND_REPO BACKEND_REPO +COMPANION_BRANCH="${COMPANION_BRANCH:-dev}" + +# The Basic-auth header is a derived credential: GitHub masks the raw token it +# minted, but not this transformation of it. Register the mask explicitly so an +# accidental trace or debug flag cannot print a working credential to the log. +AUTH_HEADER_B64="$(printf 'x-access-token:%s' "$GH_TOKEN" | base64 | tr -d '\n')" +if [ -n "${GITHUB_ACTIONS:-}" ]; then + echo "::add-mask::$AUTH_HEADER_B64" +fi + +auth_git() { + # git with the Basic auth header injected via env for this invocation only. + GIT_CONFIG_COUNT=1 \ + GIT_CONFIG_KEY_0="http.https://github.com/.extraHeader" \ + GIT_CONFIG_VALUE_0="Authorization: Basic $AUTH_HEADER_B64" \ + GIT_TERMINAL_PROMPT=0 \ + git "$@" +} + +# fetch_at