diff --git a/.github/workflows/claude-issue-to-pr.yml b/.github/workflows/claude-issue-to-pr.yml new file mode 100644 index 0000000..9bb985d --- /dev/null +++ b/.github/workflows/claude-issue-to-pr.yml @@ -0,0 +1,77 @@ +name: Claude Issue to PR + +# Thin caller of the reusable in HarperFast/ai-review-prompts. The +# single `uses:` ref pin below controls everything that moves +# together — workflow logic, auth script, agent prompt, allowed- +# labels list. Bumping the pin is the entire upgrade motion. +# +# Pre-requisites (org-level secrets, configured once on HarperFast): +# - HARPERFAST_AI_CLIENT_ID +# - HARPERFAST_AI_APP_PRIVATE_KEY +# +# Plus the per-repo / inherited: +# - ANTHROPIC_API_KEY (required) + +on: + issues: + types: [labeled] + +concurrency: + group: claude-issue-${{ github.event.issue.number }} + cancel-in-progress: false + +jobs: + work: + uses: HarperFast/ai-review-prompts/.github/workflows/_claude-issue-to-pr.yml@f6daed301f8a7ece74593506de38c6e80820b00b # main 2026-05-06 (post #11/#12/#14) + with: + # Same SHA as the `uses:` ref above. See the comment in + # claude-mention.yml for why the duplication is unavoidable. + ai-review-prompts-ref: f6daed301f8a7ece74593506de38c6e80820b00b + # Plugin repo — bun is part of the test path. + setup-bun: true + repo-specific-conventions: | + ## OAuth-specific notes + + - `config.yaml` holds the plugin entry point (`pluginModule`) + and runtime OAuth defaults. NOT doc-only — any change + there is a code/config change. + - Tests run under both Node (`npm test`) and Bun + (`bun test`); both must pass. + + ## Documentation scope boundary + + Harper maintains authoritative docs at + https://docs.harperdb.io covering core, pro, and fabric. + This repo's docs should NOT re-explain Harper mechanics + those docs already cover — they drift out of sync when the + Harper docs update. + + When writing or revising docs here: + + - Document what's specific to THIS component — env var + names, config shape, setup flow, integration API. + - For anything not component-specific (deployment mechanics, + runtime env var handling, Fabric configuration, core + Harper behavior, SQL, replication), LINK to the Harper + docs rather than re-explaining. + pre-commit-validation: | + **`config.yaml` is NOT doc-only** — it holds the plugin + entry point (`pluginModule`) and runtime OAuth defaults. + Any change touching `config.yaml` requires the full + validation path regardless of the `claude-fix:*` label. + + - `claude-fix:typo` / `claude-fix:docs` (doc-only changes + to `*.md`, `docs/**`, or `package.json` + keyword/description fields): run `npm run format:check` + and `npm run lint`. Skip `npm run build` / `npm test` / + `bun test` — they are not affected and waste turns. + - `claude-fix:deps` / `claude-fix:bug` or any change that + touches code or `config.yaml`: run + `npm run build && npm run lint && npm run format:check && npm test` + and `bun test`. Fix anything that fails. + + When in doubt, err toward the fuller validation. + secrets: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + HARPERFAST_AI_CLIENT_ID: ${{ secrets.HARPERFAST_AI_CLIENT_ID }} + HARPERFAST_AI_APP_PRIVATE_KEY: ${{ secrets.HARPERFAST_AI_APP_PRIVATE_KEY }} diff --git a/.github/workflows/claude-mention.yml b/.github/workflows/claude-mention.yml new file mode 100644 index 0000000..05190c7 --- /dev/null +++ b/.github/workflows/claude-mention.yml @@ -0,0 +1,86 @@ +name: Claude Mention Handler + +# Thin caller of the reusable in HarperFast/ai-review-prompts. The +# single `uses:` ref pin below controls everything that moves +# together — workflow logic, parse + auth scripts, agent prompt. +# Bumping the pin is the entire upgrade motion. +# +# Pre-requisites (org-level secrets, configured once on HarperFast): +# - HARPERFAST_AI_CLIENT_ID (the App's Client ID, like Iv23li…) +# - HARPERFAST_AI_APP_PRIVATE_KEY (.pem file contents) +# +# Plus the per-repo / inherited: +# - ANTHROPIC_API_KEY (required) + +on: + issue_comment: + types: [created] + pull_request_review_comment: + types: [created] + +concurrency: + group: claude-mention-${{ github.event.issue.number || github.event.pull_request.number }} + cancel-in-progress: false + +jobs: + mention: + uses: HarperFast/ai-review-prompts/.github/workflows/_claude-mention.yml@f6daed301f8a7ece74593506de38c6e80820b00b # main 2026-05-06 (post #11/#12/#14) + with: + # Same SHA as the `uses:` ref above. The reusable uses this to + # check out HarperFast/ai-review-prompts (parse + auth scripts) + # at the same ref as the workflow logic itself. + # + # The duplication is unavoidable: reusable workflows can't + # introspect their own ref (`github.workflow_ref` resolves to + # the CALLER's ref in `workflow_call` context), and `uses: …@` + # is parsed literally so we can't interpolate a variable. + ai-review-prompts-ref: f6daed301f8a7ece74593506de38c6e80820b00b + # Plugin repo — opt into bun setup so the agent can run + # `bun test` and `bun run …` for repo-specific scripts. + setup-bun: true + repo-specific-conventions: | + ## OAuth-specific notes + + - `config.yaml` holds the plugin entry point (`pluginModule`) + and runtime OAuth defaults. NOT doc-only — any change there + is a code/config change requiring full validation. + - Tests run under both Node (`npm test`) and Bun (`bun test`); + both must pass before opening / pushing to a PR. + + ## Documentation scope boundary + + Harper maintains authoritative docs at + https://docs.harperdb.io covering core, pro, and fabric. + This repo's docs should NOT re-explain Harper mechanics + those docs already cover — they drift out of sync when the + Harper docs update. + + When writing or revising docs here: + + - Document what's specific to THIS component — env var + names, config shape, setup flow, integration API. + - For anything not component-specific (deployment mechanics, + runtime env var handling, Fabric configuration, core + Harper behavior, SQL, replication), LINK to the Harper + docs rather than re-explaining. + pre-commit-validation: | + Scale validation to the kind of change you made: + + - Doc-only change (only `*.md`, `docs/**`, or `package.json` + keyword/description edits): run `npm run format:check` and + `npm run lint`. Do NOT run `npm run build` / `npm test` / + `bun test` — they are not affected and waste turns. + + `config.yaml` is NOT doc-only — it holds the plugin entry + point (`pluginModule`) and runtime OAuth defaults. Treat + any change there as a code/config change. + - Code or config change that affects behavior: run `npm ci` + first (deps aren't pre-installed in this workflow), then + `npm run build && npm run lint && npm run format:check && npm test` + and `bun test`. Fix anything that fails. + + When in doubt, err toward the fuller validation. + secrets: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + HARPERFAST_AI_CLIENT_ID: ${{ secrets.HARPERFAST_AI_CLIENT_ID }} + HARPERFAST_AI_APP_PRIVATE_KEY: ${{ secrets.HARPERFAST_AI_APP_PRIVATE_KEY }} diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index 98b7a1a..43f66b9 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -1,8 +1,24 @@ name: Claude PR Review +# Thin caller of the reusable in HarperFast/ai-review-prompts. The single +# `uses:` ref pin below controls everything that moves together — workflow +# logic, layer files, bash scripts, auth-gate behavior. Bumping the pin +# is the entire upgrade motion. +# +# Pre-requisites (org-level secrets, configured once on HarperFast): +# - HARPERFAST_AI_CLIENT_ID (the App's Client ID, like Iv23li…) +# - HARPERFAST_AI_APP_PRIVATE_KEY (.pem file contents) +# +# Plus the per-repo / inherited: +# - ANTHROPIC_API_KEY (required) +# - AI_REVIEW_LOG_TOKEN (optional — if set, threads each run +# into a per-PR issue in HarperFast/ai-review-log) + on: pull_request: - types: [opened, synchronize, reopened] + # `labeled` admits the `claude-review` label gesture for + # bot-authored PRs (renovate, dependabot). See ai-review-prompts#38. + types: [opened, synchronize, reopened, labeled] concurrency: group: claude-review-${{ github.event.pull_request.number }} @@ -10,176 +26,54 @@ concurrency: jobs: review: - # Only review PRs authored by HarperFast org members / collaborators. - # External PRs are not auto-reviewed — a maintainer can opt one in later - # via an @claude mention (handled by a separate workflow). - if: >- - contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), - github.event.pull_request.author_association) - runs-on: ubuntu-latest - timeout-minutes: 10 + uses: HarperFast/ai-review-prompts/.github/workflows/_claude-review.yml@533f841137114315b0b4d02dabf367e376d9922e # main 2026-05-21 (post #42 — log-script counts from "N blockers found" line + tolerates markdown wrapping, fixing Claude-recap titles undercounting; _claude-review.yml itself unchanged) + # Caller-side permissions, scoped at the calling-job level (NOT + # workflow-level — that placement caps the reusable's per-job + # grants below what they need and breaks the workflow at startup; + # see ai-review-prompts#39/#40 for the incident). Union of what + # the reusable's `authorize` (`contents: read`) and `review` + # (`contents: read + pull-requests: write + id-token: write`) + # jobs declare. GitHub's rule: caller's GITHUB_TOKEN permissions + # can only be DOWNGRADED (not elevated) by the called workflow, + # so the caller must grant at least the union the reusable needs. + # Reference: https://docs.github.com/en/actions/reference/workflows-and-actions/reusing-workflow-configurations permissions: contents: read pull-requests: write - id-token: write # required by claude-code-action even with API-key auth - - steps: - - name: Checkout - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - with: - fetch-depth: 0 - - - name: Claude review - id: claude-review - uses: anthropics/claude-code-action@c3d45e8e941e1b2ad7b278c57482d9c5bf1f35b3 # v1.0.99 - with: - anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - show_full_output: true # TEMP: keep on during calibration so tool denials are visible; revert once reviews run cleanly - claude_args: | - --model claude-sonnet-4-6 - --max-turns 8 - --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Read,Grep,Glob" - prompt: | - REPO: ${{ github.repository }} - PR NUMBER: ${{ github.event.pull_request.number }} - - The PR branch is already checked out in the current working directory. - - ## Tools - - For file inspection use the `Read`, `Grep`, and `Glob` tools. - Do NOT use `cat`, `head`, `tail`, `grep`, `ls`, or `find` - via Bash — those commands are not allowed and waste turns. - Do NOT run `bun test`, `npm test`, or any other script - that executes PR code — the PR's tests are already checked - separately. - - The only allowed Bash commands are: - - `gh pr view` / `gh pr diff` — inspect the PR (already run - at start, you can re-invoke if needed) - - `gh pr comment` — post the final review comment - - Read `CLAUDE.md` in the repo root first — it has the project - overview, conventions, and (importantly) a "Non-Obvious Gotchas" - section covering Resource API v2 patterns, GenericTrackedObject - spread behavior, `tsc || true` build semantics, and security - invariants. - - ## OAuth-specific things to check - - - CSRF state tokens present on every OAuth flow; 10-minute expiry - is enforced; state is single-use - - Tokens are never logged and never returned in HTTP responses - - Redirect URI validation on callback endpoints - - Provider-of-record enforcement (cross-provider CSRF protection - should redirect with error, not 403) - - Session field preservation across token refresh (`provider`, - `providerConfigId`, `providerType`) - - Resource API v2 conventions (`static loadAsInstance = false`, - instance methods) - - `GenericTrackedObject` spread gotcha — if code uses `{...obj}` - on a session field, flag it - - New endpoints include auth checks and context validation - - Path length validation (≤ 2048 chars) where user input can - reach a path - - ## Review scope and style - - Report **blocker-severity findings only**. Blockers are: - - Correctness bugs - - Security issues (token exposure, missing CSRF, auth bypass, - unvalidated redirect/path, injection) - - Broken contracts (public API behavior change without migration) - - Missing tests for new behavior or fixed bugs - - Documentation drift that would mislead integrators - - Do NOT comment on style, naming, nits, or personal preference. - Do NOT restate what the diff does. Do NOT praise. - - Cap the review at 10 findings. - - ## How to post the review - - - Use `gh pr comment` for top-level feedback (single consolidated - summary comment with all findings). - - Use `mcp__github_inline_comment__create_inline_comment` - (with `confirmed: true`) for specific code-line annotations. - - Only post GitHub comments — do NOT submit review text as SDK - messages. - - Do NOT use REQUEST_CHANGES or APPROVE during this calibration - phase. Stick to comments so the workflow never blocks or - auto-approves a merge. - - ## Output format for the top-level comment - - If you find zero blockers: post one short comment — - "No blockers found." — and stop. - - If you find blockers: post one summary comment with all findings - using this structure, one block per finding: - - ### . - - **File:** `path/to/file.ts:line` - **What:** one or two sentence description - **Why it matters:** concrete impact - **Suggested fix:** specific change, not generic advice - - No preamble. No closing summary. - - - name: Log review to ai-review-log - # Best-effort — never fail the job if logging fails - if: always() - env: - GH_TOKEN: ${{ github.token }} - AI_REVIEW_LOG_TOKEN: ${{ secrets.AI_REVIEW_LOG_TOKEN }} - PR_NUMBER: ${{ github.event.pull_request.number }} - PR_URL: ${{ github.event.pull_request.html_url }} - REVIEW_STATUS: ${{ steps.claude-review.outcome }} - run: | - set -uo pipefail - - if [ -z "${AI_REVIEW_LOG_TOKEN:-}" ]; then - echo "::warning::AI_REVIEW_LOG_TOKEN secret not set; skipping log entry." - exit 0 - fi - - # Fetch the latest comment posted by the Claude bot on this PR. - CLAUDE_BODY=$(gh pr view "$PR_NUMBER" --json comments \ - --jq '[.comments[] | select(.author.login == "claude")] | last | .body // empty') - - if [ -z "$CLAUDE_BODY" ]; then - echo "No Claude comment found on PR #$PR_NUMBER (review_status=$REVIEW_STATUS); skipping log." - exit 0 - fi - - # Title: count findings (lines starting with `### `). "No blockers" case has none. - if printf '%s' "$CLAUDE_BODY" | grep -qi '^no blockers found'; then - TITLE="[oauth] PR #$PR_NUMBER: no blockers" - else - FINDING_COUNT=$(printf '%s\n' "$CLAUDE_BODY" | grep -c '^### [0-9]' || true) - TITLE="[oauth] PR #$PR_NUMBER: ${FINDING_COUNT} finding(s) — triage pending" - fi - - BODY=$(printf '**Source:** %s\n**Repo:** oauth\n**PR:** #%s\n**Model:** claude-sonnet-4-6\n**Phase:** baseline\n**Review job status:** %s\n**Date:** %s\n\n---\n\n%s\n' \ - "$PR_URL" "$PR_NUMBER" "$REVIEW_STATUS" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$CLAUDE_BODY") - - PAYLOAD=$(jq -nc \ - --arg title "$TITLE" \ - --arg body "$BODY" \ - '{title: $title, body: $body, labels: ["repo:oauth", "verdict:pending", "phase:baseline"]}') - - HTTP=$(curl -sS -o /tmp/ai-log-resp.json -w '%{http_code}' -X POST \ - -H "Authorization: Bearer $AI_REVIEW_LOG_TOKEN" \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - https://api.github.com/repos/HarperFast/ai-review-log/issues \ - -d "$PAYLOAD") - - if [ "$HTTP" -ge 200 ] && [ "$HTTP" -lt 300 ]; then - ISSUE_URL=$(jq -r '.html_url' /tmp/ai-log-resp.json) - echo "Logged review to $ISSUE_URL" - else - echo "::warning::ai-review-log POST failed (HTTP $HTTP):" - cat /tmp/ai-log-resp.json - fi + id-token: write + with: + # Same SHA as the `uses:` ref above. The reusable uses this to + # check out HarperFast/ai-review-prompts (layer files + bash + # scripts) at the same ref as the workflow logic itself — keeps + # the upgrade motion atomic. + # + # The duplication is unavoidable: reusable workflows can't + # introspect their own ref (`github.workflow_ref` resolves to the + # CALLER's ref in `workflow_call` context), and `uses: …@` + # is parsed literally so we can't interpolate a variable. + ai-review-prompts-ref: 533f841137114315b0b4d02dabf367e376d9922e + review-layers: | + universal + harper/common + harper/v5 + repo-type/plugin + repo-specific-checks: | + ## Repo-specific checks (OAuth plugin) + + On top of the layered scope above, these are OAuth-only + semantics not covered by the shared layers: + + - CSRF state tokens present on every OAuth flow; 10-minute + expiry is enforced; state is single-use + - Redirect URI validation on callback endpoints + - Provider-of-record enforcement (cross-provider CSRF + protection should redirect with error, not 403) + - Session field preservation across token refresh + (`provider`, `providerConfigId`, `providerType`) + - Path length validation (≤ 2048 chars) where user input + can reach a path + secrets: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + AI_REVIEW_LOG_TOKEN: ${{ secrets.AI_REVIEW_LOG_TOKEN }} + HARPERFAST_AI_CLIENT_ID: ${{ secrets.HARPERFAST_AI_CLIENT_ID }} + HARPERFAST_AI_APP_PRIVATE_KEY: ${{ secrets.HARPERFAST_AI_APP_PRIVATE_KEY }} diff --git a/.github/workflows/gemini-review.yml b/.github/workflows/gemini-review.yml new file mode 100644 index 0000000..0f0d67f --- /dev/null +++ b/.github/workflows/gemini-review.yml @@ -0,0 +1,84 @@ +name: Gemini PR Review + +# Thin caller of the Gemini reusable in HarperFast/ai-review-prompts. +# Runs in parallel with claude-review.yml so the two reviewers can be +# compared on the same PRs. +# +# Layer inputs and `repo-specific-checks:` MIRROR claude-review.yml +# in this repo. Output comparability between the two providers +# depends on them seeing the same review scope — keep them in sync +# when bumping the pin or editing the checks block. +# +# Pre-requisites: +# - HARPERFAST_AI_CLIENT_ID (org-level App Client ID) +# - HARPERFAST_AI_APP_PRIVATE_KEY (org-level App private key) +# - GEMINI_API_KEY (per-repo; optional — missing +# key cleanly skips the review +# with a workflow notice) +# - AI_REVIEW_LOG_TOKEN (optional — threads each run +# into a per-(PR, provider) issue +# in HarperFast/ai-review-log +# with `provider:gemini` label) + +on: + pull_request: + types: [opened, synchronize, reopened] + +concurrency: + # Different group key from claude-review so the two providers can + # run in parallel on the same PR. cancel-in-progress is per-group, + # so a synchronize push cancels the in-flight Gemini run without + # touching the Claude run (and vice versa). + group: gemini-review-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + review: + uses: HarperFast/ai-review-prompts/.github/workflows/_gemini-review.yml@533f841137114315b0b4d02dabf367e376d9922e # main 2026-05-21 (post #42 — continuity preamble now mandatory when prior findings exist so Gemini updates narrate the round, plus log-script count-line fix) + # Caller-side permissions, scoped at the calling-job level (NOT + # workflow-level — that placement caps the reusable's per-job + # grants below what they need and breaks the workflow at startup; + # see ai-review-prompts#39/#40 for the incident). Union of what + # the reusable's `authorize` (`contents: read`) and `review` + # (`contents: read + pull-requests: write + id-token: write`) + # jobs declare. GitHub's rule: caller's GITHUB_TOKEN permissions + # can only be DOWNGRADED (not elevated) by the called workflow, + # so the caller must grant at least the union the reusable needs. + # Mirrors claude-review.yml in this repo — surfaced as a finding + # by Gemini's own review on PR #88. + # Reference: https://docs.github.com/en/actions/reference/workflows-and-actions/reusing-workflow-configurations + permissions: + contents: read + pull-requests: write + id-token: write + with: + # Same SHA as the `uses:` ref above. See claude-review.yml + # in this repo for why the duplication is unavoidable + # (reusable workflows can't introspect their own ref in + # workflow_call context). + ai-review-prompts-ref: 533f841137114315b0b4d02dabf367e376d9922e + review-layers: | + universal + harper/common + harper/v5 + repo-type/plugin + repo-specific-checks: | + ## Repo-specific checks (OAuth plugin) + + On top of the layered scope above, these are OAuth-only + semantics not covered by the shared layers: + + - CSRF state tokens present on every OAuth flow; 10-minute + expiry is enforced; state is single-use + - Redirect URI validation on callback endpoints + - Provider-of-record enforcement (cross-provider CSRF + protection should redirect with error, not 403) + - Session field preservation across token refresh + (`provider`, `providerConfigId`, `providerType`) + - Path length validation (≤ 2048 chars) where user input + can reach a path + secrets: + GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} + AI_REVIEW_LOG_TOKEN: ${{ secrets.AI_REVIEW_LOG_TOKEN }} + HARPERFAST_AI_CLIENT_ID: ${{ secrets.HARPERFAST_AI_CLIENT_ID }} + HARPERFAST_AI_APP_PRIVATE_KEY: ${{ secrets.HARPERFAST_AI_APP_PRIVATE_KEY }} diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 8ca8955..8126180 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -13,14 +13,14 @@ jobs: strategy: matrix: - node-version: [20, 22, 24] + node-version: [22, 24] steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Setup Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v4 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: ${{ matrix.node-version }} cache: 'npm' @@ -34,25 +34,19 @@ jobs: - name: Check formatting run: npm run format:check - # Coverage was failing to run on Node 20, so run it only on 22+ - - name: Run tests with coverage (Node 22+) - if: matrix.node-version >= 22 + - name: Run tests with coverage run: npm run test:coverage - - name: Run tests (Node 20) - if: matrix.node-version == 20 - run: npm run test:node-twenty - test-bun: name: Test with Bun runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Setup Bun - uses: oven-sh/setup-bun@v2 + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 with: bun-version: latest diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f3808ab..4c416c2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,7 +18,7 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Setup Node.js - uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: '22' registry-url: 'https://registry.npmjs.org' diff --git a/.github/workflows/validate-caller-workflows.yml b/.github/workflows/validate-caller-workflows.yml new file mode 100644 index 0000000..16e5bfa --- /dev/null +++ b/.github/workflows/validate-caller-workflows.yml @@ -0,0 +1,33 @@ +name: Validate caller workflows + +# Thin caller of HarperFast/ai-review-prompts' `_validate-caller-workflows.yml`. +# Validates `.github/workflows/claude-*.yml` caller files for: +# +# * Shadow jobs — a non-`uses:` job (or a `uses:` outside `HarperFast/`) +# alongside the legit reusable call would run with the caller's +# permissions WITHOUT going through the auth gate. Fail-closed. +# * Mutable refs in `uses:` or `with.ai-review-prompts-ref` — both +# must pin to a 40-char SHA. +# +# Runs on every PR and every push to `main` — no `paths:` filter. +# A required status check that only fires on workflow-touching PRs +# stays permanently pending on every other PR (GitHub has no +# "required if it runs" semantic). The validator's runtime is +# trivial (yq-parses a few caller files), so unconditional firing +# is the right trade-off for satisfiability. +# +# Make this `validate` job a REQUIRED status check on `main`. + +on: + pull_request: + push: + branches: [main] + +jobs: + validate: + uses: HarperFast/ai-review-prompts/.github/workflows/_validate-caller-workflows.yml@f6daed301f8a7ece74593506de38c6e80820b00b # main 2026-05-06 (post #11/#12/#14) + with: + # Same SHA as the `uses:` ref above — the reusable uses this to + # check out the validator script at the matching version. Same + # SHA-twice pattern as the other caller workflows in this repo. + ai-review-prompts-ref: f6daed301f8a7ece74593506de38c6e80820b00b diff --git a/scripts/v4-routing-repro.mjs b/scripts/v4-routing-repro.mjs new file mode 100644 index 0000000..87efa1a --- /dev/null +++ b/scripts/v4-routing-repro.mjs @@ -0,0 +1,387 @@ +#!/usr/bin/env node +/** + * v4-routing-repro.mjs — regression guard for the symptom Dawson reported + * on 2026-05-22 (CM/Studio Okta SSO): + * + * "providerName is 'oauth', not the {configId} described, that isn't passed" + * + * Counterpart of integrationTests/path-segment-routing.test.ts on `main` + * (Harper v5), built for the v1.x line where CM consumes us against + * harperdb ^4.7.28. + * + * Spawns harperdb v4 against a temp fixture that configures two providers — + * one literally named "oauth" (decoy) and one named "oac-oauth-tenant" (the + * substring "oauth" inside this name also guards against an over-aggressive + * stripping regression, not just the prefix-extraction case). Fires + * /oauth/oac-oauth-tenant/login and inspects the resulting redirect. + * + * Outcomes: + * - Redirect to tenant.test/authorize with the tenant client_id → parseRoute + * correctly extracted the URL path segment. This is the current behavior + * on harperdb 4.7.19 + the in-tree plugin source — Dawson's diagnosis + * does not reproduce on this stack. + * - Anything else (404, decoy redirect, malformed URL) → the assertion + * fails; investigate parseRoute / the resource dispatch path. + * + * Side effects — harperdb's installer rewrites ~/.harperdb/hdb_boot_properties.file + * to point at the temp ROOTPATH. The script backs that file up (into the + * run's tempRoot) before launch and restores it in a single cleanup path + * shared by the normal `finally` block and the signal/exit handlers, so an + * existing local `harperdb` install is left intact across runs. If the + * script is SIGKILL'd between backup and the harperdb install actually + * touching the file, the backup is collocated with tempRoot and may be + * left orphaned in $TMPDIR until OS cleanup; check there for recovery. + * + * Not wired into CI. Run manually: + * node scripts/v4-routing-repro.mjs + * + * Set KEEP_TEMP=1 to preserve the temp dir for inspection. + */ +import { mkdtempSync, writeFileSync, rmSync, existsSync, copyFileSync } from 'node:fs'; +import { tmpdir, homedir } from 'node:os'; +import { join, dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { spawn, spawnSync } from 'node:child_process'; + +const BOOT_PROPS_PATH = join(homedir(), '.harperdb', 'hdb_boot_properties.file'); + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const harperBin = join(repoRoot, 'node_modules', '.bin', 'harperdb'); +const tscBin = join(repoRoot, 'node_modules', '.bin', 'tsc'); + +// Pin the fixture's harperdb peer to the exact version we're spawning so +// `npm install` doesn't resolve a drifted version from the registry. +// Sourced from node_modules/harperdb/package.json at script-write time; +// kept in sync via the SOURCE-OF-TRUTH comment so a Renovate / manual bump +// of the repo's harperdb dep has a single human-readable place to update. +const HARPERDB_PIN = '4.7.19'; // SOURCE-OF-TRUTH: node_modules/harperdb/package.json + +const TENANT_CLIENT_ID = 'oauth-tenant-client-id'; +const DECOY_CLIENT_ID = 'decoy-oauth-client-id'; + +const CONFIG_YAML = ` +rest: true + +'@harperfast/oauth': + package: '@harperfast/oauth' + providers: + oauth: + provider: generic + clientId: ${DECOY_CLIENT_ID} + clientSecret: decoy-secret + authorizationUrl: 'http://decoy.test/authorize' + tokenUrl: 'http://decoy.test/token' + userInfoUrl: 'http://decoy.test/userinfo' + oac-oauth-tenant: + provider: generic + clientId: ${TENANT_CLIENT_ID} + clientSecret: tenant-secret + authorizationUrl: 'http://tenant.test/authorize' + tokenUrl: 'http://tenant.test/token' + userInfoUrl: 'http://tenant.test/userinfo' +`.trimStart(); + +const PACKAGE_JSON = JSON.stringify( + { + name: 'oauth-v4-routing-repro', + private: true, + type: 'module', + // Pin to the exact harperdb being spawned by this script so the + // fixture's npm install can't resolve a drifted registry version + // of the >=4.6.0 peer range. + devDependencies: { harperdb: HARPERDB_PIN }, + }, + null, + 2 +); + +function run(cmd, args, opts = {}) { + const result = spawnSync(cmd, args, { stdio: 'inherit', ...opts }); + if (result.status !== 0) { + throw new Error(`${cmd} ${args.join(' ')} exited ${result.status}`); + } +} + +function log(...args) { + console.log('[v4-repro]', ...args); +} + +async function waitForReady(harperProc, timeoutMs) { + const deadline = Date.now() + timeoutMs; + return new Promise((resolve, reject) => { + let buf = ''; + const onData = (chunk) => { + const s = String(chunk); + buf += s; + process.stdout.write(s); + if (/successfully started/i.test(buf)) { + cleanup(); + resolve(); + } + }; + const onExit = (code) => { + cleanup(); + reject(new Error(`harperdb exited prematurely with code ${code}`)); + }; + const onError = (err) => { + cleanup(); + reject(new Error(`harperdb spawn error: ${err.message}`)); + }; + const onTimeout = () => { + cleanup(); + reject(new Error(`harperdb did not become ready within ${timeoutMs}ms`)); + }; + const timer = setTimeout(onTimeout, deadline - Date.now()); + const cleanup = () => { + clearTimeout(timer); + harperProc.stdout?.off('data', onData); + harperProc.stderr?.off('data', onData); + harperProc.off('exit', onExit); + harperProc.off('error', onError); + }; + harperProc.stdout?.on('data', onData); + harperProc.stderr?.on('data', onData); + harperProc.once('exit', onExit); + harperProc.once('error', onError); + }); +} + +// Single cleanup state — populated as we go, consulted by both the +// finally block and the signal handlers so they don't redundantly try +// to restore boot props or kill harperdb. +const cleanupState = { + bootBackupPath: null, // set once tempRoot exists + bootState: 'pending', // 'pending' | 'existed' | 'absent' + tempRoot: null, + harperProc: null, + cleaned: false, +}; + +function backupBootProps() { + // If a backup file already exists at this run's path it means tempRoot + // is being reused (impossible — mkdtemp is fresh each run), so existsSync + // here should always be false on a healthy invocation. The check is a + // belt-and-suspenders guard. + if (existsSync(cleanupState.bootBackupPath)) { + log(`WARNING: backup at ${cleanupState.bootBackupPath} already exists; treating as authoritative`); + cleanupState.bootState = 'existed'; + return; + } + if (existsSync(BOOT_PROPS_PATH)) { + copyFileSync(BOOT_PROPS_PATH, cleanupState.bootBackupPath); + log(`saved boot props to ${cleanupState.bootBackupPath}`); + cleanupState.bootState = 'existed'; + return; + } + log('no existing boot props file to back up'); + cleanupState.bootState = 'absent'; +} + +function restoreBootProps() { + const { bootBackupPath, bootState } = cleanupState; + try { + if (bootState === 'existed') { + if (bootBackupPath && existsSync(bootBackupPath)) { + copyFileSync(bootBackupPath, BOOT_PROPS_PATH); + log(`restored boot props from backup`); + } else { + log(`WARNING: backup at ${bootBackupPath} missing — leaving boot props as-is`); + } + } else if (bootState === 'absent') { + // harperdb's installer creates the file even if it wasn't there + // before. Remove it to restore the original "no boot props" state. + rmSync(BOOT_PROPS_PATH, { force: true }); + log(`removed boot props (none existed before)`); + } + } catch (err) { + log(`WARNING: failed to restore boot props: ${err.message}. Backup may be at ${bootBackupPath}`); + } +} + +function cleanup() { + if (cleanupState.cleaned) return; + cleanupState.cleaned = true; + + const { harperProc, tempRoot } = cleanupState; + + if (harperProc && harperProc.exitCode === null) { + log('killing harperdb...'); + try { + harperProc.kill('SIGINT'); + } catch { + // Already exited between the check and the kill — fine. + } + // Best-effort follow-up SIGKILL; we can't await an async timer from + // a synchronous signal handler, so accept the brief inconsistency. + setTimeout(() => { + if (harperProc.exitCode === null) { + try { + harperProc.kill('SIGKILL'); + } catch { + // Already exited; nothing to do. + } + } + }, 1000); + } + + restoreBootProps(); + + if (tempRoot && !process.env.KEEP_TEMP) { + log(`cleaning up ${tempRoot}`); + try { + rmSync(tempRoot, { recursive: true, force: true }); + } catch (err) { + log(`WARNING: failed to remove ${tempRoot}: ${err.message}`); + } + } else if (tempRoot) { + log(`KEEP_TEMP set; leaving ${tempRoot}`); + } +} + +async function main() { + if (!existsSync(harperBin)) { + throw new Error(`harperdb binary not found at ${harperBin}. Run "npm install" in the repo root first.`); + } + if (!existsSync(tscBin)) { + throw new Error(`tsc binary not found at ${tscBin}. Run "npm install" in the repo root first.`); + } + + cleanupState.tempRoot = mkdtempSync(join(tmpdir(), 'oauth-v4-repro-')); + cleanupState.bootBackupPath = join(cleanupState.tempRoot, 'hdb_boot_properties.file.bak'); + const componentsRoot = join(cleanupState.tempRoot, 'components'); + const componentAppDir = join(componentsRoot, 'app'); + const hdbRoot = join(cleanupState.tempRoot, 'hdb-root'); + log(`temp root: ${cleanupState.tempRoot}`); + + // Save the current boot props before harperdb's installer rewrites them. + // Done outside the try so the backup is in place if anything below throws + // before harperdb spawns. + backupBootProps(); + + let exitCode = 1; + + try { + // 1. Build the plugin with a stricter invocation than `npm run build` + // (which is `tsc || true` — silent failures can leave stale dist/). + // Clear dist/ first so a TS emission failure is visible by the + // missing dist/index.js after. + log('rebuilding plugin (strict)...'); + rmSync(join(repoRoot, 'dist'), { recursive: true, force: true }); + run(tscBin, [], { cwd: repoRoot }); + const distEntry = join(repoRoot, 'dist', 'index.js'); + if (!existsSync(distEntry)) { + throw new Error(`build did not emit ${distEntry}`); + } + + // 2. npm pack the local plugin into the temp dir + log('packing local plugin...'); + const packResult = spawnSync('npm', ['pack', '--pack-destination', cleanupState.tempRoot, '--json'], { + cwd: repoRoot, + encoding: 'utf8', + }); + if (packResult.status !== 0) { + console.error(packResult.stderr); + throw new Error(`npm pack failed (exit ${packResult.status})`); + } + const tarballName = JSON.parse(packResult.stdout)[0].filename; + const tarballPath = join(cleanupState.tempRoot, tarballName); + + // 3. Set up the fixture component directly under components/app + // (avoids a wasteful cp -r of node_modules later). + log(`writing fixture to ${componentAppDir}...`); + run('mkdir', ['-p', componentAppDir]); + writeFileSync(join(componentAppDir, 'config.yaml'), CONFIG_YAML); + writeFileSync(join(componentAppDir, 'package.json'), PACKAGE_JSON); + + log('installing plugin into fixture component dir...'); + run('npm', ['install', '--no-save', '--no-audit', '--no-fund', tarballPath], { cwd: componentAppDir }); + + // 4. Spawn harperdb with CLI-arg config — modeled on + // @harperfast/integration-testing@0.3.0 (v5). Same argument names + // exist in v4's bundled binary (ROOTPATH, HDB_ADMIN_*, HTTP_PORT, + // OPERATIONSAPI_*, etc.). + const httpPort = 19926; + const opsPort = 19925; + const hostname = '127.0.0.1'; + const args = [ + `--ROOTPATH=${hdbRoot}`, + `--TC_AGREEMENT=yes`, + `--HDB_ADMIN_USERNAME=admin`, + `--HDB_ADMIN_PASSWORD=Abc1234!`, + `--DEFAULTS_MODE=dev`, + `--REPLICATION_HOSTNAME=localhost`, + `--HTTP_PORT=${hostname}:${httpPort}`, + `--OPERATIONSAPI_NETWORK_PORT=${hostname}:${opsPort}`, + `--NODE_HOSTNAME=${hostname}`, + `--THREADS_COUNT=1`, + `--LOGGING_LEVEL=debug`, + `--LOGGING_STDSTREAMS=true`, + `--CLUSTERING_ENABLED=false`, + `--COMPONENTSROOT=${componentsRoot}`, + ]; + + log(`spawning: ${harperBin} ${args.join(' ')}`); + cleanupState.harperProc = spawn(harperBin, args); + + log('waiting for ready...'); + await waitForReady(cleanupState.harperProc, 60_000); + + // 5. Fire the request and check the redirect. + const url = `http://${hostname}:${httpPort}/oauth/oac-oauth-tenant/login`; + log(`fetching ${url}`); + const response = await fetch(url, { redirect: 'manual' }); + log(`status: ${response.status}`); + const location = response.headers.get('location'); + log(`location: ${location}`); + + if (response.status !== 302 || !location) { + throw new Error(`expected 302 redirect, got ${response.status} (location: ${location})`); + } + + const redirectUrl = new URL(location); + const target = redirectUrl.origin + redirectUrl.pathname; + const clientId = redirectUrl.searchParams.get('client_id'); + + log(`redirect target: ${target}`); + log(`client_id: ${clientId}`); + + if (target === 'http://tenant.test/authorize' && clientId === TENANT_CLIENT_ID) { + log('PASS: routed to tenant provider — parseRoute extracted the URL path segment'); + exitCode = 0; + } else if (target === 'http://decoy.test/authorize' && clientId === DECOY_CLIENT_ID) { + log( + 'FAIL: routed to DECOY provider — parseRoute extracted literal "oauth" segment (bug confirmed on this stack)' + ); + exitCode = 1; + } else { + log(`UNEXPECTED: target=${target} client_id=${clientId}`); + exitCode = 1; + } + } catch (err) { + log('error:', err.message); + exitCode = 1; + } finally { + cleanup(); + // Give the post-cleanup SIGKILL timer a moment to land before exiting. + await new Promise((r) => setTimeout(r, 1100)); + } + + process.exit(exitCode); +} + +// Route signal exits through the shared cleanup path so harperdb is killed, +// tempRoot is removed, and boot props are restored before we leave. +for (const sig of ['SIGINT', 'SIGTERM', 'SIGHUP', 'SIGQUIT']) { + process.on(sig, () => { + cleanup(); + const code = sig === 'SIGINT' ? 130 : sig === 'SIGTERM' ? 143 : sig === 'SIGHUP' ? 129 : 131; + // Brief delay so the cleanup's async timer can fire if needed. + setTimeout(() => process.exit(code), 1100).unref(); + }); +} + +main().catch((err) => { + console.error('[v4-repro] fatal:', err); + cleanup(); + process.exit(1); +});