diff --git a/.cargo/config.toml b/.cargo/config.toml index 9c9e20f089..5df8ea0c15 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -20,3 +20,7 @@ rustflags = ["-C", "target-feature=+aes,+sse2,+ssse3,+sse4.1,+sse4.2"] [target.x86_64-pc-windows-msvc] rustflags = ["-C", "target-feature=+aes,+sse2,+ssse3,+sse4.1,+sse4.2"] + +[env] +# test threads get the 16 MB stack of the server's runtime threads (THREAD_STACK_SIZE); debug async code outgrows 2 MB +RUST_MIN_STACK = "16777216" diff --git a/.github/workflows/ai-code-review.yml b/.github/workflows/ai-code-review.yml index 975fbc32ec..3114220bd4 100644 --- a/.github/workflows/ai-code-review.yml +++ b/.github/workflows/ai-code-review.yml @@ -68,7 +68,6 @@ jobs: - name: anthropic provider_id: anthropic-review model_id: claude-sonnet-5-5 - model_variant: high api_key_env: ANTHROPIC_REVIEW_API_KEY marker: "" steps: @@ -109,6 +108,11 @@ jobs: fi merge_base="$(gh api "repos/$REPO/compare/$base_sha...$head_sha" --jq '.merge_base_commit.sha' || true)" echo "merge_base=$merge_base" >> "$GITHUB_OUTPUT" + { + echo "author=$(jq -r '.user.login // ""' <<<"$pr_json")" + echo "changed_files=$(jq -r '.changed_files // ""' <<<"$pr_json")" + echo "commit_count=$(jq -r '.commits // ""' <<<"$pr_json")" + } >> "$GITHUB_OUTPUT" echo "REVIEW_RUN=true" >> "$GITHUB_ENV" # If minting fails (app not installed, bad key) the steps below fall back to github.token. @@ -149,6 +153,37 @@ jobs: .claude/skills/ui-architect .claude/skills/eslint-error-handling + # Bills nothing and posts nothing; a hand-started run, a failed listing or a failed check always reviews. + - name: Skip PRs with nothing to review + if: env.REVIEW_RUN == 'true' && github.event_name == 'pull_request_target' + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ steps.pr.outputs.number }} + PR_AUTHOR: ${{ steps.pr.outputs.author }} + # The listings are checked against these, as both endpoints stop at a cap however far they are paged. + EXPECTED_FILES: ${{ steps.pr.outputs.changed_files }} + EXPECTED_COMMITS: ${{ steps.pr.outputs.commit_count }} + run: | + set -uo pipefail + list="$RUNNER_TEMP/pr-files.tsv" + commits="$RUNNER_TEMP/pr-commits.tsv" + # One line per file with its rename source, so code moved under docs/ is not taken for docs and the count can be checked. + if ! gh api --paginate "repos/$REPO/pulls/$PR_NUMBER/files?per_page=100" --jq '.[] | [.filename, (.previous_filename // "")] | @tsv' > "$list"; then + echo "::warning::Could not list the PR's files; reviewing." + : > "$list" + fi + # One line per commit: author login, committer login, signature verified; a missing field lists as empty and reviews. + if ! gh api --paginate "repos/$REPO/pulls/$PR_NUMBER/commits?per_page=100" --jq '.[] | [(.author.login // ""), (.committer.login // ""), ((.commit.verification.verified // "") | tostring)] | @tsv' > "$commits"; then + echo "::warning::Could not list the PR's commits; reviewing." + : > "$commits" + fi + reason="$(node engine/scripts/ai-review/skip-check.mjs "$list" "$commits")" || reason="" + if [ -n "$reason" ]; then + echo "::notice::PR #$PR_NUMBER needs no review ($reason); skipped." + echo "REVIEW_RUN=false" >> "$GITHUB_ENV" + fi + # Paired OSS+ENT changes are reviewed once, by the o2-enterprise workflow, which pulls this PR's diff in. - name: Defer to a paired o2-enterprise PR if: env.REVIEW_RUN == 'true' && !inputs.ignore_paired @@ -291,10 +326,12 @@ jobs: FORCE_FULL: ${{ inputs.force_full || 'false' }} REVIEW_PROVIDER_ID: ${{ matrix.provider.provider_id }} REVIEW_MODEL_ID: ${{ matrix.provider.model_id }} - REVIEW_MODEL_VARIANT: ${{ matrix.provider.model_variant }} + # The effort is set here only, so the variable is the single source; each value must be a variant in opencode.ci.jsonc. + REVIEW_MODEL_VARIANT: ${{ vars.AI_REVIEW_EFFORT || 'high' }} + REVIEW_COORDINATOR_VARIANT: ${{ vars.AI_REVIEW_COORDINATOR_EFFORT || vars.AI_REVIEW_EFFORT || 'high' }} REVIEW_API_KEY_ENV: ${{ matrix.provider.api_key_env }} REVIEW_MARKER: ${{ matrix.provider.marker }} - # opencode.ci.jsonc: every tool (shell, edit, webfetch) allowed with no prompts; "default" is the read-only opencode.jsonc. + # opencode.ci.jsonc: file tools plus a read-only shell allowlist, no web or sub-agent tools, no prompts; "default" is the read-only opencode.jsonc. REVIEW_OPENCODE_CONFIG: ci-full # Empty under the github.token fallback: any workflow a same-repo PR adds can also post as github-actions[bot]. REVIEW_BOT_LOGIN: ${{ steps.app_token.outputs.token && format('{0}[bot]', steps.app_token.outputs.app-slug) || '' }} @@ -313,6 +350,8 @@ jobs: # Optional ceiling on automatic reviews per PR; unset means no cap, so each new commit gets one review. The manual input bypasses it for one run. REVIEW_MAX_PASSES: ${{ vars.AI_REVIEW_MAX_PASSES }} REVIEW_IGNORE_PASS_LIMIT: ${{ inputs.ignore_pass_limit || 'false' }} + # A hand-started run reviews even a diff the last review already read. + REVIEW_MANUAL_RUN: ${{ github.event_name == 'workflow_dispatch' }} # Independent reviewer sessions per area (1-3), merged by the coordinator; one cold pass misses findings a second catches. REVIEW_COLD_PASSES: ${{ vars.AI_REVIEW_COLD_PASSES || '2' }} # An unknown value means off, so a typo can never start approving. diff --git a/.github/workflows/api-testing.yml b/.github/workflows/api-testing.yml index 62a2425b6c..27694dcfd1 100644 --- a/.github/workflows/api-testing.yml +++ b/.github/workflows/api-testing.yml @@ -654,6 +654,8 @@ jobs: # 2. Nothing API-relevant changed → nothing ran → allow. if [ "${{ needs.check_changes.outputs.has_changes }}" != "true" ]; then echo "✅ No API-relevant changes — suite not required." + echo "::notice title=API tests skipped::No API-relevant changes — suite did not run." + echo "### ⏭️ API suite skipped — no API-relevant changes in this PR, so no tests ran." >> "$GITHUB_STEP_SUMMARY" exit 0 fi diff --git a/.github/workflows/api_bugfix_regression.yml b/.github/workflows/api_bugfix_regression.yml index 12b47b3b39..1336c9f5ac 100644 --- a/.github/workflows/api_bugfix_regression.yml +++ b/.github/workflows/api_bugfix_regression.yml @@ -156,7 +156,7 @@ jobs: name: "api_regression / ${{ matrix.config.label }}" needs: [check_main_changed, build_api_binary] if: ${{ needs.check_main_changed.outputs.should_run == 'true' }} - runs-on: ubicloud-standard-4 + runs-on: eks-openobserve-standard-4 permissions: actions: read contents: read diff --git a/.github/workflows/audit-checker.yml b/.github/workflows/audit-checker.yml index 734c1c1635..e473505950 100644 --- a/.github/workflows/audit-checker.yml +++ b/.github/workflows/audit-checker.yml @@ -27,7 +27,7 @@ jobs: steps: - uses: actions/checkout@v5 - uses: ./.github/actions/github-auth - - uses: taiki-e/install-action@1e29709c0ec63babbe8f266397a465f29a825f91 # v2.87.31 + - uses: taiki-e/install-action@1919c4a95903d2c33e4a221102edca87309fc14b # v2.89.0 env: GITHUB_TOKEN: ${{ github.token }} with: diff --git a/.github/workflows/bug-checker.yml b/.github/workflows/bug-checker.yml index 954d14e815..3179517f1a 100644 --- a/.github/workflows/bug-checker.yml +++ b/.github/workflows/bug-checker.yml @@ -144,7 +144,7 @@ jobs: fetch-depth: 1 - name: Analyze bugs with Claude - uses: anthropics/claude-code-action@16f9f71ab9c490faf367f94dd294897edc25be33 # v1.0.261 + uses: anthropics/claude-code-action@d535f5172dec02b204866b393d6b20b9b2aaf98d # v1.0.266 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} diff --git a/.github/workflows/build-pr-image.yml b/.github/workflows/build-pr-image.yml index 2e99917b89..695c129505 100644 --- a/.github/workflows/build-pr-image.yml +++ b/.github/workflows/build-pr-image.yml @@ -342,7 +342,7 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - - uses: int128/docker-manifest-create-action@a118e9fe12455375d4810aa318c20e6dba77288e # v2.29.0 + - uses: int128/docker-manifest-create-action@d6ac360314337bbfb870747d9616b83118acb330 # v2.30.0 with: tags: | public.ecr.aws/zinclabs/openobserve-dev:${{ needs.build_frontend.outputs.GIT_TAG }}-${{ needs.build_frontend.outputs.GIT_HASH}} @@ -350,7 +350,7 @@ jobs: public.ecr.aws/zinclabs/openobserve-dev:${{ needs.build_frontend.outputs.GIT_TAG }}-${{ needs.build_frontend.outputs.GIT_HASH}}-amd64 public.ecr.aws/zinclabs/openobserve-dev:${{ needs.build_frontend.outputs.GIT_TAG }}-${{ needs.build_frontend.outputs.GIT_HASH}}-arm64 - - uses: int128/docker-manifest-create-action@a118e9fe12455375d4810aa318c20e6dba77288e # v2.29.0 + - uses: int128/docker-manifest-create-action@d6ac360314337bbfb870747d9616b83118acb330 # v2.30.0 with: tags: | ghcr.io/openobserve/openobserve-dev:${{ needs.build_frontend.outputs.GIT_TAG }}-${{ needs.build_frontend.outputs.GIT_HASH}} diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 46e8ca44f5..10807ad51c 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -85,6 +85,6 @@ jobs: - name: Run Claude Code id: claude - uses: anthropics/claude-code-action@16f9f71ab9c490faf367f94dd294897edc25be33 # v1.0.261 + uses: anthropics/claude-code-action@d535f5172dec02b204866b393d6b20b9b2aaf98d # v1.0.266 with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} diff --git a/.github/workflows/ent-e2e-gate.yml b/.github/workflows/ent-e2e-gate.yml index 3935d9259a..17ccb58214 100644 --- a/.github/workflows/ent-e2e-gate.yml +++ b/.github/workflows/ent-e2e-gate.yml @@ -14,9 +14,9 @@ name: Playwright E2E Crosscheck # · open sister ENT PR (same-named branch) → DEFER to that PR's checks (link it, don't build here) # · sister ENT branch only (no PR) → build the enterprise binary against that branch # · PR base is a release line (not main) with a same-named ENT branch → build against THAT branch -# · no ENT counterpart → build against ENT main, testing the PR merged into -# OSS main (refs/pull/N/merge; branch head if not mergeable) -# then POLL and mirror pass/fail. A lost test runner is re-run once before it is reported. +# · no ENT counterpart → build against ENT main, testing the merge commit SHA tied to +# the event's head SHA (the head SHA itself if there is none) +# then POLL and mirror pass/fail. Only a lost test runner is re-run, up to twice, before it is reported. # # A single STICKY comment per PR (found by a hidden marker) reports running → passed / failed, lists the # failing specs, and links the sister ENT PR/branch + the ENT run. A build failure is attributed by the @@ -31,65 +31,35 @@ name: Playwright E2E Crosscheck # vars.ENT_E2E_GATE_DISABLED — 'true' kill switch: gate passes without dispatching. on: + # Opens, reopens, pushes, base changes and the gate's own labels run it; any other label or edit only mirrors its current result. pull_request: - types: [synchronize, labeled, unlabeled, ready_for_review, converted_to_draft] - # Runs on label changes of a ready-for-ci PR and on approval; pushes run only while approved. - pull_request_review: - types: [submitted] + types: [opened, reopened, synchronize, edited, labeled, unlabeled, ready_for_review, converted_to_draft] # A required check must also report on merge-queue commits; the gate already passed on the PR head. merge_group: -# PR-scoped: a new push/label supersedes the previous gate attempt for the same PR. +# PR-scoped: a new push or gate label supersedes the previous gate attempt for the same PR. concurrency: - # A non-approval review does nothing, so it must not cancel a running gate. - group: playwright-e2e-crosscheck-${{ github.event.pull_request.number || github.ref }}${{ github.event_name == 'pull_request_review' && github.event.review.state != 'approved' && format('-{0}', github.run_id) || '' }} + # Other labels (auto-requeue-* and the like) and title/body edits only mirror the gate's result, so they must not cancel a running gate; the newest mirror is enough. + group: playwright-e2e-crosscheck-${{ github.event.pull_request.number || github.ref }}${{ (((github.event.action == 'labeled' || github.event.action == 'unlabeled') && github.event.label.name != 'ready-for-ci' && github.event.label.name != 'skip-ent-e2e') || (github.event.action == 'edited' && !github.event.changes.base)) && '-mirror' || '' }} cancel-in-progress: true permissions: contents: read - pull-requests: write + pull-requests: read jobs: - trigger: - # reviewDecision of a stacked PR is judged by the stack's merge rules, so it is what the merge will require. - if: ${{ github.event_name != 'merge_group' && (contains(github.event.pull_request.labels.*.name, 'ready-for-ci') && github.event.pull_request.draft == false) && ((github.event_name == 'pull_request_review' && github.event.review.state == 'approved') || github.event_name == 'pull_request') }} - runs-on: ubuntu-latest - timeout-minutes: 5 - permissions: - pull-requests: read - outputs: - dispatch: ${{ steps.check.outputs.dispatch }} - steps: - - id: check - env: - GH_TOKEN: ${{ github.token }} - GH_REPO: ${{ github.repository }} - PR: ${{ github.event.pull_request.number }} - ACTION: ${{ github.event_name == 'pull_request' && github.event.action || '' }} - LABEL: ${{ github.event.label.name }} - run: | - # Any label change cancels the running gate, so it must start a replacement; without approval only label events and ready_for_review do. - if [ "$ACTION" = "labeled" ] || [ "$ACTION" = "unlabeled" ] || [ "$ACTION" = "ready_for_review" ]; then - echo "Triggered by $ACTION; approval not required." - echo "dispatch=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - DECISION=$(gh pr view "$PR" --json reviewDecision --jq .reviewDecision) - echo "reviewDecision: $DECISION" - DISPATCH=false - if [ "$DECISION" = "APPROVED" ]; then DISPATCH=true; fi - echo "dispatch=$DISPATCH" >> "$GITHUB_OUTPUT" - playwright-e2e-crosscheck: - needs: trigger - # A pre-merge gate: runs when ready-for-ci is added or the PR is approved, and on pushes only while approved. - if: ${{ !cancelled() && (github.event_name == 'merge_group' || needs.trigger.outputs.dispatch == 'true') }} + # Without ready-for-ci, or on a draft, the required ci-gate status stays pending, so skipping here cannot let a PR merge. + if: ${{ github.event_name == 'merge_group' || (contains(github.event.pull_request.labels.*.name, 'ready-for-ci') && github.event.pull_request.draft == false) }} name: Playwright E2E Crosscheck runs-on: ubuntu-latest - # ~5 min to find the ENT run + up to ~75 min to poll. Generous on purpose: normal is ~20 min, - # but a cold Rust cache can push the ENT build+run toward its own timeouts — don't false-fail - # the gate ("timed out") while ENT is still legitimately running. A lost runner adds one re-run. - timeout-minutes: 150 + permissions: + contents: read + pull-requests: read + checks: read + actions: read + # Starting (5) + the first run's budget (165) + two lost-runner re-runs (80 each, plus 1 to see the new attempt), from the poll budgets below. + timeout-minutes: 340 steps: - name: Triage — relevant paths? opted out? id: triage @@ -99,6 +69,7 @@ jobs: HAS_SKIP: ${{ contains(github.event.pull_request.labels.*.name, 'skip-ent-e2e') }} DISABLED: ${{ vars.ENT_E2E_GATE_DISABLED }} IS_FORK: ${{ github.event.pull_request.head.repo.fork }} + MIRROR: ${{ ((github.event.action == 'labeled' || github.event.action == 'unlabeled') && github.event.label.name != 'ready-for-ci' && github.event.label.name != 'skip-ent-e2e') || (github.event.action == 'edited' && !github.event.changes.base) }} run: | set -euo pipefail if [ "${{ github.event_name }}" = "merge_group" ]; then @@ -109,9 +80,7 @@ jobs: echo "::notice::ENT_E2E_GATE_DISABLED=true — passing without dispatch." echo "mode=pass" >> "$GITHUB_OUTPUT"; exit 0 fi - # Fork PRs: head branch isn't in openobserve/openobserve and the PAT isn't available to - # forked-PR runs, so the ENT engine can't check it out. Pass with a notice; a maintainer - # re-runs from an internal branch if enterprise validation is needed. + # A fork's branch is not in this repo and its runs get no PAT, so the engine cannot check it out. if [ "$IS_FORK" = "true" ]; then echo "::notice::Fork PR — Playwright E2E Crosscheck can't run (no PAT / branch not in main repo). Passing." echo "mode=pass" >> "$GITHUB_OUTPUT"; exit 0 @@ -119,7 +88,12 @@ jobs: # Relevant = any changed file under web/ or tests/ui-testing/ FILES=$(gh api "repos/${{ github.repository }}/pulls/$PR/files" --paginate -q '.[].filename') - if printf '%s\n' "$FILES" | grep -qE '^(web/|tests/ui-testing/)'; then + CHANGED=$(gh api "repos/${{ github.repository }}/pulls/$PR" -q .changed_files) + # The files API lists at most 3000 files, so a list it cut short cannot prove the PR is irrelevant. + if [ "$CHANGED" -ge 3000 ] || [ "$CHANGED" -gt "$(grep -c . <<<"$FILES")" ]; then + RELEVANT=true + # A here-string, not a pipe: grep -q exits early and the writer's SIGPIPE would fail the pipeline under pipefail. + elif grep -qE '^(web/|tests/ui-testing/)' <<<"$FILES"; then RELEVANT=true else RELEVANT=false @@ -131,6 +105,8 @@ jobs: elif [ "$HAS_SKIP" = "true" ]; then echo "::warning::PR carries 'skip-ent-e2e' — enterprise gate explicitly opted out (auditable)." echo "mode=skip" >> "$GITHUB_OUTPUT" + elif [ "$MIRROR" = "true" ]; then + echo "mode=mirror" >> "$GITHUB_OUTPUT" else echo "mode=run" >> "$GITHUB_OUTPUT" fi @@ -139,6 +115,60 @@ jobs: if: steps.triage.outputs.mode == 'pass' || steps.triage.outputs.mode == 'skip' run: echo "✅ Playwright E2E Crosscheck not required for this PR." + - name: Mirror — report the gate's current result for this head + if: steps.triage.outputs.mode == 'mirror' + env: + GH_TOKEN: ${{ github.token }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + RUN_ID: ${{ github.run_id }} + GATE_ON: ${{ contains(github.event.pull_request.labels.*.name, 'ready-for-ci') && github.event.pull_request.draft == false }} + run: | + set -euo pipefail + CHECKS="repos/${{ github.repository }}/commits/$HEAD_SHA/check-runs?check_name=Playwright%20E2E%20Crosscheck&filter=all&per_page=100" + OWN="" + for _ in 1 2 3 4 5; do + OWN=$(gh api --paginate "$CHECKS" | jq -r --arg u "/actions/runs/$RUN_ID/" '.check_runs[] | select(.details_url | contains($u)) | .id' | sort -n | tail -n 1) && [ -n "$OWN" ] && break + sleep 10 + done + [ -n "$OWN" ] || { echo "::error::Could not find this job's own check run."; exit 1; } + # Only older checks count: the newest check of this name is the required one, so a later gate run overrides this mirror anyway. + ERRORS=0; IGNORE="[]" + api_error() { + ERRORS=$((ERRORS + 1)) + if [ "$ERRORS" -ge 10 ]; then echo "::error::The GitHub API kept failing — cannot report the gate's result."; exit 1; fi + sleep 30 + } + while :; do + # An API error must not read as "no result", which would pass. + if ! LIST=$(gh api --paginate "$CHECKS" | jq -r --argjson own "$OWN" --argjson ignore "$IGNORE" \ + '.check_runs[] | select(.id < $own and .conclusion != "skipped" and (.id as $i | $ignore | index($i) | not)) | "\(.id) \(.status) \(.conclusion // "")"'); then + api_error; continue + fi + read -r ID STATUS CONCL <<<"$(sort -n <<<"$LIST" | tail -n 1)" + if [ -z "$ID" ]; then + # This check is the newest, so passing here would make the required check green without the gate ever running. + if [ "$GATE_ON" = "true" ]; then + echo "::error::No Playwright E2E Crosscheck run exists for $HEAD_SHA, so there is no result to report. Re-apply the 'ready-for-ci' label or push a commit to run the gate." + exit 1 + fi + echo "::notice::No gate result on $HEAD_SHA yet — nothing to mirror." + exit 0 + fi + # Mirrors and failed triages share this check name, so only a check whose Run step started is a gate result; an unfinished one may still start it. + if ! RAN=$(gh api "repos/${{ github.repository }}/actions/jobs/$ID" -q '[.steps[]? | select(.status == "in_progress" or (.conclusion != null and .conclusion != "skipped")) | .name | split(" — ")[0]] | join(" ")'); then + api_error; continue + fi + if [[ " $RAN " == *" Run "* ]]; then + if [ "$STATUS" = "completed" ]; then break; fi + elif [[ " $RAN " == *" Mirror "* ]] || [ "$STATUS" = "completed" ]; then + IGNORE=$(jq -c --argjson i "$ID" '. + [$i]' <<<"$IGNORE"); continue + fi + # This check is newer than the running gate, so it must end with the gate's result, not before it. + ERRORS=0; sleep 30 + done + echo "::notice::Mirroring gate check $ID on $HEAD_SHA: $CONCL." + [ "$CONCL" = "success" ] || { echo "::error::The gate's current result on this head is '$CONCL'."; exit 1; } + - name: Run — dispatch ENT engine, poll, mirror result if: steps.triage.outputs.mode == 'run' env: @@ -152,23 +182,30 @@ jobs: set -euo pipefail REPO="openobserve/o2-enterprise"; WF="oss-pr-ent-gate.yml" MARKER="" + GATE_LOGIN=$(gh api user -q .login 2>/dev/null || echo "") - # Upsert ONE sticky gate comment per PR (located by the hidden marker) so status updates in - # place — pass AND fail — instead of spamming a fresh comment each run. + # One display-only status comment per PR; --paginate prints one array per page, so take the first matching id. sticky() { - local body="$1" cid - cid=$(gh api "repos/openobserve/openobserve/issues/$PR/comments" --paginate \ - -q "[.[] | select(.body|contains(\"$MARKER\")) | .id][0]" 2>/dev/null || echo "") - if [ -n "$cid" ] && [ "$cid" != "null" ]; then - gh api -X PATCH "repos/openobserve/openobserve/issues/comments/$cid" -f body="$body" >/dev/null 2>&1 || true + STICKY_ID=${STICKY_ID:-$(gh api "repos/openobserve/openobserve/issues/$PR/comments" --paginate 2>/dev/null \ + | jq -r --arg login "$GATE_LOGIN" --arg marker "$MARKER" '.[] | select(.user.login == $login and (.body | contains($marker))) | .id' | head -n 1 || true)} + if [ -n "$STICKY_ID" ]; then + gh api -X PATCH "repos/openobserve/openobserve/issues/comments/$STICKY_ID" -f body="$1" >/dev/null 2>&1 || true else - gh api -X POST "repos/openobserve/openobserve/issues/$PR/comments" -f body="$body" >/dev/null 2>&1 || true + STICKY_ID=$(gh api -X POST "repos/openobserve/openobserve/issues/$PR/comments" -f body="$1" -q .id 2>/dev/null || echo "") fi } + cannot_start() { + sticky "### 🚫 Playwright E2E Crosscheck — could not start - # Sister ENT resolution. An OPEN same-named ENT PR is the live counterpart — let ITS checks - # own enterprise validation; we DEFER (don't build here), just link. Only a branch (no PR) → - # build against that branch. Neither → the PR's base-line ENT branch if it exists, else main. + $MARKER + $1 The gate stays red until it runs — re-push or re-label to retry. + + ${SISTER_NOTE:-}" + echo "::error::$1" + exit 1 + } + + # An open same-named ENT PR runs these specs itself, so the gate defers to its checks. SISTER_PR=$(gh pr list --repo "$REPO" --head "$HEAD_REF" --state open --json number -q '.[0].number' 2>/dev/null || echo "") if [ -n "$SISTER_PR" ] && [ "$SISTER_PR" != "null" ]; then sticky "### ⏭️ Playwright E2E Crosscheck — handled by the sister ENT PR @@ -179,15 +216,12 @@ jobs: exit 0 fi - # No sister branch/PR → build against the PR's base line, not blindly main. A PR onto a - # release branch (e.g. branch-v1.0.0) must build against the ENT branch of the same name; - # ENT main would fail to compile the release-line OSS code. Fall back to main only when - # the base is main or has no ENT counterpart. - if gh api "repos/$REPO/branches/$HEAD_REF" >/dev/null 2>&1; then + # A release-line PR needs the same-named ENT branch, and only plain names may reach API paths ("#", "?", "%" retarget them). + if [[ "$HEAD_REF" =~ ^[A-Za-z0-9._/-]+$ ]] && gh api "repos/$REPO/branches/$HEAD_REF" >/dev/null 2>&1; then ENT_CODE_REF="$HEAD_REF" SISTER_NOTE="Built against the matching ENT branch \`$HEAD_REF\` (no ENT PR yet)." echo "::notice::ENT branch '$HEAD_REF' exists (no PR) — building against it." - elif [ "$BASE_REF" != "main" ] && [ "$BASE_REF" != "master" ] && gh api "repos/$REPO/branches/$BASE_REF" >/dev/null 2>&1; then + elif [ "$BASE_REF" != "main" ] && [ "$BASE_REF" != "master" ] && [[ "$BASE_REF" =~ ^[A-Za-z0-9._/-]+$ ]] && gh api "repos/$REPO/branches/$BASE_REF" >/dev/null 2>&1; then ENT_CODE_REF="$BASE_REF" SISTER_NOTE="No same-named ENT branch — built against the PR's base line \`$BASE_REF\` on ENT." echo "::notice::No ENT branch '$HEAD_REF'; PR targets '$BASE_REF' which exists on ENT — building against it." @@ -197,60 +231,74 @@ jobs: echo "::notice::No ENT counterpart for '$HEAD_REF' (base '$BASE_REF') — building against main." fi + # The run title names the exact ENT code and engine commits, so the gate can tell its own run from any other. + sha_of() { + local sha + for _ in 1 2 3; do + sha=$(gh api "repos/$REPO/commits/$1" -q .sha 2>/dev/null || true) + if [[ "$sha" =~ ^[0-9a-f]{40}$ ]]; then echo "$sha"; return 0; fi + sleep 5 + done + } + ENT_SHA=$(sha_of "$ENT_CODE_REF") + [ -n "$ENT_SHA" ] || cannot_start "Could not resolve ENT \`$ENT_CODE_REF\` to a commit." + # Against ENT main, test the PR merged into OSS main: a stale branch would miss OSS code ENT main already uses. - OSS_REF="$HEAD_REF" + TESTED_SHA="$HEAD_SHA" if [ "$ENT_CODE_REF" = "main" ] && [ "$BASE_REF" = "main" ]; then - MERGEABLE="" + MERGEABLE=""; MERGE_SHA=""; PR_HEAD="" for _ in 1 2 3 4 5; do - MERGEABLE=$(gh api "repos/openobserve/openobserve/pulls/$PR" -q '.mergeable' 2>/dev/null || echo "") + read -r MERGEABLE MERGE_SHA PR_HEAD < <(gh api "repos/openobserve/openobserve/pulls/$PR" -q '"\(.mergeable) \(.merge_commit_sha // "-") \(.head.sha)"' 2>/dev/null || echo "") if [ "$MERGEABLE" = "true" ] || [ "$MERGEABLE" = "false" ]; then break; fi sleep 6 done - if [ "$MERGEABLE" = "true" ]; then - OSS_REF="refs/pull/$PR/merge" + # The merge commit belongs to this push only when the same response reports this head. + if [ "$MERGEABLE" = "true" ] && [[ "$MERGE_SHA" =~ ^[0-9a-f]{40}$ ]] && [ "$PR_HEAD" = "$HEAD_SHA" ]; then + TESTED_SHA="$MERGE_SHA" SISTER_NOTE="$SISTER_NOTE Tested as this PR merged into OSS \`main\`." else - echo "::warning::PR #$PR is not cleanly mergeable into main (mergeable=$MERGEABLE) — testing the branch head as-is." + echo "::warning::PR #$PR has no merge commit for head $HEAD_SHA (mergeable=$MERGEABLE, head=$PR_HEAD) — testing the branch head as-is." fi fi - # Stamp the time BEFORE dispatch so correlation can reject stale prior runs for this PR. + # The engine checks out oss_branch, so a commit SHA there makes it test exactly what its title names. + MATCH="Playwright E2E Crosscheck — OSS PR #$PR (" + # The engine commit is whatever ENT_REF points at when the run starts, so it is read from the title, not predicted. + TITLE="${MATCH}$TESTED_SHA) oss=$HEAD_SHA ent=$ENT_SHA engine=" + newest_run() { + gh run list --repo "$REPO" --workflow "$WF" --event workflow_dispatch -L 40 --json databaseId,displayTitle,createdAt,event 2>/dev/null \ + | jq -r --arg t "$TITLE" --arg old "${MATCH}$TESTED_SHA)" --argjson min "$1" \ + '[.[] | select(.event == "workflow_dispatch" and ((.displayTitle | startswith($t) and (ltrimstr($t) | test("^[0-9a-f]{40}$"))) or .displayTitle == $old) and (.createdAt | fromdateiso8601) >= $min)] | sort_by(.databaseId) | last | .databaseId // empty' 2>/dev/null || true + } DISPATCH_TS=$(date -u +%s) - echo "::notice::Dispatching ENT engine ($REPO@$ENT_REF, code=$ENT_CODE_REF, oss=$OSS_REF) for OSS PR #$PR ($HEAD_REF)…" + echo "::notice::Dispatching ENT engine ($REPO@$ENT_REF, code=$ENT_CODE_REF@$ENT_SHA, oss=$TESTED_SHA) for OSS PR #$PR ($HEAD_REF)…" gh workflow run "$WF" --repo "$REPO" --ref "$ENT_REF" \ - -f oss_branch="$OSS_REF" \ + -f oss_branch="$TESTED_SHA" \ + -f oss_head_ref="$HEAD_REF" \ -f oss_pr="$PR" \ -f oss_sha="$HEAD_SHA" \ - -f enterprise_ref="$ENT_CODE_REF" - - # Correlate by PR-scoped run-name ("Playwright E2E Crosscheck — OSS PR # ("; the "(" boundary keeps - # #12 from matching #123) AND createdAt >= dispatch time (minus 60s clock-skew), so a stale - # prior run for this PR is never picked. Take the NEWEST qualifying run. - MATCH="OSS PR #$PR (" - MIN_TS=$((DISPATCH_TS - 60)) + -f enterprise_ref="$ENT_SHA" RUN_ID="" - sleep 8 # let the dispatched run register before the first lookup - for i in $(seq 1 30); do # ~5 min to appear - RUN_ID=$(gh run list --repo "$REPO" --workflow "$WF" -L 40 \ - --json databaseId,displayTitle,createdAt \ - -q "[.[] | select(.displayTitle | contains(\"$MATCH\")) | select((.createdAt|fromdateiso8601) >= $MIN_TS)] | sort_by(.createdAt) | last | .databaseId" 2>/dev/null || echo "") - if [ -n "$RUN_ID" ] && [ "$RUN_ID" != "null" ]; then break; fi - sleep 10 + sleep 8 + for _ in $(seq 1 30); do + RUN_ID=$(newest_run "$((DISPATCH_TS - 60))") + if [[ "$RUN_ID" =~ ^[0-9]+$ ]]; then break; fi + RUN_ID=""; sleep 10 done - if [ -z "$RUN_ID" ] || [ "$RUN_ID" = "null" ]; then - sticky "### 🚫 Playwright E2E Crosscheck — could not start - - $MARKER - Dispatching the enterprise e2e engine failed or the run never appeared (check \`ORG_ADMIN_TOKEN\` / \`ENT_GATE_REF=$ENT_REF\`). The gate stays red until it runs — re-push or re-label to retry. - - $SISTER_NOTE" - echo "::error::Could not locate the dispatched ENT run within 5 min — failing the gate (unverified)." - exit 1 - fi + [ -n "$RUN_ID" ] || cannot_start "Dispatching the enterprise e2e engine failed or the run never appeared within 5 min (check \`ORG_ADMIN_TOKEN\` / \`ENT_GATE_REF=$ENT_REF\`)." RUN_URL="https://github.com/$REPO/actions/runs/$RUN_ID" echo "::notice::Tracking ENT run: $RUN_URL" + report_pass() { + sticky "### ✅ Playwright E2E Crosscheck — passed + + $MARKER + The enterprise-only e2e specs passed against this PR. $SISTER_NOTE + + [ENT run]($RUN_URL)" + echo "::notice::✅ Enterprise e2e specs passed against this PR." + } sticky "### 🔄 Playwright E2E Crosscheck — running $MARKER @@ -259,15 +307,17 @@ jobs: [ENT run]($RUN_URL)" - # A lost runner is infra, not the PR: re-run the failed ENT job once before reporting it. - ATTEMPT=1 + # A lost runner is infra, not the PR: re-run only the lost ENT jobs, up to 3 attempts of the run in all. + MAX_ATTEMPTS=3; RETRIES=0 + # Poll budgets follow the engine's job timeouts: subset 10 + build 75 + shard 45 + aggregate 10 + report 10 min, plus 15 min of runner queueing. + SHARD_MIN=45; RUN_BUDGET_MIN=$((10 + 75 + SHARD_MIN + 10 + 10 + 15)); RERUN_BUDGET_MIN=$((SHARD_MIN + 10 + 10 + 15)) + DEADLINE=$((SECONDS + RUN_BUDGET_MIN * 60)) while :; do - # Poll to completion (~75 min budget — covers a cold-cache ENT build+run worst case). STATUS=""; CONCL="" - for i in $(seq 1 300); do # 300 * 15s = 75 min + while :; do read -r STATUS CONCL < <(gh run view "$RUN_ID" --repo "$REPO" \ --json status,conclusion -q '.status + " " + (.conclusion // "")' 2>/dev/null || echo "unknown ") - if [ "$STATUS" = "completed" ]; then break; fi + if [ "$STATUS" = "completed" ] || [ "$SECONDS" -ge "$DEADLINE" ]; then break; fi sleep 15 done @@ -281,50 +331,68 @@ jobs: exit 1 fi - if [ "$CONCL" = "success" ]; then - sticky "### ✅ Playwright E2E Crosscheck — passed + if [ "$CONCL" = "success" ]; then report_pass; exit 0; fi + + # A run of this same test cancels ours (engine concurrency), so the gate follows it; anything else never finished the specs. + if [ "$CONCL" = "cancelled" ] || [ "$CONCL" = "skipped" ] || [ -z "$CONCL" ]; then + NEXT=$(newest_run "$((DISPATCH_TS - 60))") + if [[ "$NEXT" =~ ^[0-9]+$ ]] && [ "$NEXT" -gt "$RUN_ID" ]; then + echo "::notice::ENT run $RUN_ID was cancelled by run $NEXT of this same test — following it." + RUN_ID="$NEXT"; RUN_URL="https://github.com/$REPO/actions/runs/$RUN_ID"; DEADLINE=$((SECONDS + RUN_BUDGET_MIN * 60)) + continue + fi + sticky "### 🚫 Playwright E2E Crosscheck — enterprise run was cancelled, re-run needed $MARKER - The enterprise-only e2e specs passed against this PR. $SISTER_NOTE + The [enterprise e2e run]($RUN_URL) was cancelled (conclusion: \`${CONCL:-none}\`) and no newer run of this same test replaced it, so the specs did not finish. Re-run the failed job of this check. - [ENT run]($RUN_URL)" - echo "::notice::✅ Enterprise e2e specs passed against this PR." - exit 0 + $SISTER_NOTE" + echo "::error::ENT run concluded '${CONCL:-none}' with no newer run of this same test — failing the gate (unverified)." + exit 1 fi - # cancelled/skipped/empty ≈ a newer push superseded this run (engine cancel-in-progress). - # NOT a failure — the newer run is authoritative and updates the sticky itself. Defer. - if [ "$CONCL" = "cancelled" ] || [ "$CONCL" = "skipped" ] || [ -z "$CONCL" ]; then - echo "::notice::ENT run concluded '$CONCL' (likely superseded by a newer push) — deferring, not failing." - exit 0 - fi - - # Genuine failure — distinguish an enterprise BUILD failure (compile) from SPEC failures, - # and for specs list the failing files from the engine's failed-specs artifact. + # A build failure and spec failures need different fixes, so the report tells them apart. BUILD_CONCL=$(gh run view "$RUN_ID" --repo "$REPO" --json jobs -q '.jobs[]|select(.name=="build_binary")|.conclusion' 2>/dev/null || echo "") # Single-job re-runs spread shard results over attempts, so each test job counts by its latest attempt; an aggregate counts only without shards. TEST_JOBS=$(gh api --paginate "repos/$REPO/actions/runs/$RUN_ID/jobs?filter=all&per_page=100" --jq '.jobs[]' 2>/dev/null | jq -s '[.[]|select(.name=="ent_gate_tests" or (.name|startswith("ent_gate_tests (")))] | group_by(.name) | map(max_by(.run_attempt)) | (map(select(.name|startswith("ent_gate_tests (")))) as $s | if ($s|length) > 0 then $s else . end' 2>/dev/null || echo '[]') TEST_CONCL=$(jq -r '[.[].conclusion]|if index("failure") then "failure" elif length > 0 then .[0] // "" else "" end' <<<"$TEST_JOBS") - # A test job that failed with no failed step lost its runner; one with a failed step had spec failures. - TEST_LOST=$(jq '[.[]|select(.conclusion=="failure" and ([.steps[]?|select(.conclusion=="failure")]|length)==0)]|length' <<<"$TEST_JOBS") - TEST_SPEC_FAILED=$(jq '[.[]|select(.conclusion=="failure" and ([.steps[]?|select(.conclusion=="failure")]|length)>0)]|length' <<<"$TEST_JOBS") + # A lost runner leaves its step running: no failed or cancelled step, and it ended before the job timeout, which cancels the step. + FAILED_JOBS=$(jq -c --argjson limit "$SHARD_MIN" '[.[] | select(.conclusion == "failure") | {id, lost: (([.steps[]? | select(.conclusion == "failure" or .conclusion == "cancelled")] | length) == 0 and .started_at != null and .completed_at != null and ((.completed_at | fromdateiso8601) - (.started_at | fromdateiso8601)) < ($limit * 60))}]' <<<"$TEST_JOBS" 2>/dev/null || echo '[]') + LOST_IDS=$(jq -r '.[] | select(.lost) | .id' <<<"$FAILED_JOBS") + TEST_LOST=$(jq '[.[] | select(.lost)] | length' <<<"$FAILED_JOBS") + TEST_SPEC_FAILED=$(jq '[.[] | select(.lost | not)] | length' <<<"$FAILED_JOBS") TEST_LOST=${TEST_LOST:-0}; TEST_SPEC_FAILED=${TEST_SPEC_FAILED:-0} + RUN_ATTEMPT=$(gh api "repos/$REPO/actions/runs/$RUN_ID" -q '.run_attempt' 2>/dev/null || echo "$MAX_ATTEMPTS") - if [ "$TEST_CONCL" = "failure" ] && [ "$TEST_LOST" != "0" ] && [ "$ATTEMPT" = "1" ]; then - ATTEMPT=2 - echo "::warning::ENT test runner was lost — re-running the failed ENT job once." - if gh run rerun "$RUN_ID" --repo "$REPO" --failed; then + # A re-run would repeat the jobs with spec failures too, so only a lost-runner-only failure is retried. + if [ "$TEST_CONCL" = "failure" ] && [ "$TEST_LOST" != "0" ] && [ "$TEST_SPEC_FAILED" = "0" ] \ + && [ "$RUN_ATTEMPT" -lt "$MAX_ATTEMPTS" ] && [ "$RETRIES" -lt $((MAX_ATTEMPTS - 1)) ]; then + RETRIES=$((RETRIES + 1)) + echo "::warning::ENT test runner lost in $TEST_LOST job(s) — re-running them (attempt $((RUN_ATTEMPT + 1)) of $MAX_ATTEMPTS)." + # The jobs API re-runs one job; a run can't take a second job re-run while the first is going, and with no spec failures --failed holds only lost jobs. + if [ "$TEST_LOST" = "1" ]; then + RERUN=(gh api -X POST "repos/$REPO/actions/jobs/$LOST_IDS/rerun") + else + RERUN=(gh run rerun "$RUN_ID" --repo "$REPO" --failed) + fi + if "${RERUN[@]}" >/dev/null; then sticky "### 🔄 Playwright E2E Crosscheck — runner lost, re-running $MARKER - The enterprise test runner stopped before the specs finished (infra, not this PR), so the failed job is being re-run automatically. + The enterprise test runner stopped before the specs finished (infra, not this PR), so the lost job is being re-run automatically (attempt $((RUN_ATTEMPT + 1)) of $MAX_ATTEMPTS). $SISTER_NOTE [ENT run]($RUN_URL)" - sleep 20 + # The poll must not read the finished attempt as the re-run's result. + for _ in $(seq 1 12); do + NOW_ATTEMPT=$(gh api "repos/$REPO/actions/runs/$RUN_ID" -q '.run_attempt' 2>/dev/null || echo "$RUN_ATTEMPT") + if [ "$NOW_ATTEMPT" -gt "$RUN_ATTEMPT" ]; then break; fi + sleep 5 + done + DEADLINE=$((SECONDS + RERUN_BUDGET_MIN * 60)) continue fi - echo "::warning::Could not re-run the ENT job — reporting the lost runner." + echo "::warning::Could not re-run the lost ENT job(s) — reporting the lost runner." fi break done @@ -334,7 +402,10 @@ jobs: rm -rf /tmp/fs; SPEC_LIST="" if [ "$TEST_CONCL" = "failure" ] && gh run download "$RUN_ID" --repo "$REPO" -n failed-specs -D /tmp/fs >/dev/null 2>&1 && [ -f /tmp/fs/failed-specs.txt ]; then - SPEC_LIST=$(sed '/^[[:space:]]*$/d; s/^/- `/; s/$/`/' /tmp/fs/failed-specs.txt) + # A comment body is capped at 65536 characters, so a huge list would make the update fail. + SPEC_LIST=$(awk '!/^[[:space:]]*$/ && n < 50 { n++; print "- `" substr($0, 1, 200) "`" }' /tmp/fs/failed-specs.txt) + SPEC_N=$(grep -cv '^[[:space:]]*$' /tmp/fs/failed-specs.txt || true) + if [ "${SPEC_N:-0}" -gt 50 ]; then SPEC_LIST="$SPEC_LIST"$'\n'"- … $((SPEC_N - 50)) more"; fi fi # Spec failures in another shard outrank a lost runner; the artifact only lists their files. @@ -342,7 +413,7 @@ jobs: sticky "### ⚠️ Playwright E2E Crosscheck — runner lost, re-run needed $MARKER - The enterprise test runner stopped before the specs finished (infra, not this PR), and the automatic re-run didn't get past it. Re-run the failed job of this check. + The enterprise test runner stopped before the specs finished (infra, not this PR), and the automatic re-runs didn't get past it. Re-run the failed job of this check. $SISTER_NOTE · [ENT run]($RUN_URL)" elif [ "$TEST_CONCL" = "failure" ]; then diff --git a/.github/workflows/js-license-checker.yml b/.github/workflows/js-license-checker.yml index 79e9814b79..79441f440d 100644 --- a/.github/workflows/js-license-checker.yml +++ b/.github/workflows/js-license-checker.yml @@ -23,7 +23,7 @@ jobs: js-license-check: # Paid CI runs only on PRs carrying the 'ready-for-ci' label; the merge queue runs unit tests on the merged code. if: ${{ github.event_name != 'pull_request' || (contains(github.event.pull_request.labels.*.name, 'ready-for-ci') && github.event.pull_request.draft == false) }} - runs-on: ubicloud-standard-4 + runs-on: eks-openobserve-standard-4 timeout-minutes: 10 steps: - name: Checkout latest code diff --git a/.github/workflows/mobile-sdk-e2e.yml b/.github/workflows/mobile-sdk-e2e.yml index fbbbc78ace..a17a9139f3 100644 --- a/.github/workflows/mobile-sdk-e2e.yml +++ b/.github/workflows/mobile-sdk-e2e.yml @@ -486,7 +486,7 @@ jobs: with: { distribution: temurin, java-version: "17" } - uses: actions/setup-node@v4 with: { node-version: "20" } - - uses: gradle/actions/setup-gradle@3f5f9adaf7d9fecd50b5935e54106014257a94e6 # v6.4.0 + - uses: gradle/actions/setup-gradle@8da46b539cd8fe6ab27ca75128919a18d757604c # v6.4.1 - name: Install Maestro run: | curl -Ls "https://get.maestro.mobile.dev" | bash diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 3412776212..3bd77dea95 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -780,18 +780,32 @@ jobs: if-no-files-found: ignore retention-days: 1 + # Playwright retries only a failed test, so a retry in a passing shard is a flaky test whose first failure the server log explains. + - name: Detect flaky tests + id: flaky + if: success() + continue-on-error: true + run: | + flaky=false + for z in tests/ui-testing/blob-report/*.zip; do + [ -f "$z" ] || continue + retries=$(unzip -p "$z" report.jsonl 2>/dev/null | grep -cE '"method":"onTestBegin".*"retry":[1-9]' || true) + if [ "${retries:-0}" -gt 0 ]; then flaky=true; echo "$z: $retries retried test attempt(s)"; fi + done + echo "flaky=$flaky" >> "$GITHUB_OUTPUT" + - name: Show OpenObserve log tail - if: failure() + if: failure() || steps.flaky.outputs.flaky == 'true' run: tail -n 200 o2.log || true - name: Upload OpenObserve log - if: failure() + if: failure() || steps.flaky.outputs.flaky == 'true' continue-on-error: true uses: actions/upload-artifact@v4 with: name: o2-log-${{ matrix.testfolder }}-attempt-${{ github.run_attempt }} path: o2.log - retention-days: 1 + retention-days: 7 if-no-files-found: ignore merge_reports: @@ -1154,6 +1168,8 @@ jobs: # 2. Nothing Playwright-relevant changed → nothing ran → allow. if [ "${{ needs.check_changes.outputs.has_changes }}" != "true" ]; then echo "✅ No Playwright-relevant changes — suite not required." + echo "::notice title=Playwright tests skipped::No Playwright-relevant changes — suite did not run." + echo "### ⏭️ Playwright suite skipped — no Playwright-relevant changes in this PR, so no tests ran." >> "$GITHUB_STEP_SUMMARY" exit 0 fi diff --git a/.github/workflows/stuck-run-reaper.yml b/.github/workflows/stuck-run-reaper.yml index 1999de9446..13c81dc154 100644 --- a/.github/workflows/stuck-run-reaper.yml +++ b/.github/workflows/stuck-run-reaper.yml @@ -53,7 +53,7 @@ jobs: env: # Comma-separated workflow file names to police (all use eks-openobserve-* runners). # Release workflows are intentionally excluded — a stuck release build is left for a human. - WORKFLOWS: "playwright.yml,playwright_bugfix_regression.yml" + WORKFLOWS: "playwright.yml,playwright_bugfix_regression.yml,unit-tests.yml,js-license-checker.yml,api_bugfix_regression.yml" DEFAULT_THRESHOLD_MINUTES: "60" # Separate, much longer cap for "zombie" runs stuck with no jobs ever dispatched # (these never held a runner, so 60m would be too aggressive — a brief GitHub diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 191ddef6fa..2f3a2c0e75 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -67,7 +67,7 @@ jobs: needs: [check_changes] if: needs.check_changes.outputs.has_backend_changes == 'true' runs-on: - labels: ubicloud-standard-8 + labels: eks-openobserve-standard-8 permissions: contents: read steps: @@ -92,6 +92,10 @@ jobs: - name: Install Protoc run: bash ./.github/protoc.sh + # The EKS runner image has no libclang, which bindgen needs to build custom-labels. + - name: Install libclang + run: sudo apt-get update && sudo apt-get install -y libclang-dev + - name: Simulate building frontend code run: mkdir -p web/dist && touch web/dist/index.html @@ -141,7 +145,7 @@ jobs: toolchain: nightly-2026-10-24 targets: x86_64-unknown-linux-gnu - - uses: taiki-e/install-action@1e29709c0ec63babbe8f266397a465f29a825f91 # v2.87.31 + - uses: taiki-e/install-action@1919c4a95903d2c33e4a221102edca87309fc14b # v2.89.0 env: GITHUB_TOKEN: ${{ github.token }} with: @@ -210,7 +214,7 @@ jobs: toolchain: nightly-2026-10-24 targets: x86_64-unknown-linux-gnu - - uses: taiki-e/install-action@1e29709c0ec63babbe8f266397a465f29a825f91 # v2.87.31 + - uses: taiki-e/install-action@1919c4a95903d2c33e4a221102edca87309fc14b # v2.89.0 env: GITHUB_TOKEN: ${{ github.token }} with: @@ -272,7 +276,7 @@ jobs: matrix: shard: [1, 2, 3, 4] runs-on: - labels: ubicloud-standard-8 + labels: eks-openobserve-standard-8 permissions: contents: read @@ -290,7 +294,7 @@ jobs: - name: Restore node_modules id: nm - uses: actions/cache/restore@v4 + uses: actions/cache/restore@v6 with: path: web/node_modules key: node-modules-${{ runner.os }}-node24-${{ hashFiles('web/package-lock.json', 'web/package.json', 'web/patches/**') }} @@ -304,7 +308,7 @@ jobs: # Only main saves it: a PR-saved cache is visible to that PR alone and fills the 10 GB repo cache. - name: Save node_modules if: github.ref == 'refs/heads/main' && steps.nm.outputs.cache-hit != 'true' - uses: actions/cache/save@v4 + uses: actions/cache/save@v6 with: path: web/node_modules key: node-modules-${{ runner.os }}-node24-${{ hashFiles('web/package-lock.json', 'web/package.json', 'web/patches/**') }} @@ -333,7 +337,8 @@ jobs: needs: [check_changes, ui_unit_test_shards] if: always() && needs.check_changes.outputs.has_ui_changes == 'true' timeout-minutes: 20 - runs-on: ubuntu-latest + runs-on: + labels: eks-openobserve-standard-4 permissions: contents: read @@ -354,7 +359,7 @@ jobs: - name: Restore node_modules id: nm - uses: actions/cache/restore@v4 + uses: actions/cache/restore@v6 with: path: web/node_modules key: node-modules-${{ runner.os }}-node24-${{ hashFiles('web/package-lock.json', 'web/package.json', 'web/patches/**') }} @@ -379,7 +384,7 @@ jobs: needs: [check_changes] if: needs.check_changes.outputs.has_ui_changes == 'true' runs-on: - labels: ubicloud-standard-8 + labels: eks-openobserve-standard-8 permissions: contents: read diff --git a/.gitignore b/.gitignore index 25a38a8e65..2515c90a65 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ .history /.idea/ .vscode/ +/.zed/ # ---------- node / web ---------- node_modules/ diff --git a/Cargo.lock b/Cargo.lock index 451cb5b4fb..e6f6936ecc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8430,6 +8430,7 @@ dependencies = [ "tikv-jemalloc-ctl", "time", "tokio", + "tokio-util", "tonic", "tracing", "tracing-opentelemetry", diff --git a/Cargo.toml b/Cargo.toml index 015d785250..b7ecf476e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -586,6 +586,7 @@ tiktoken-rs = "0.9.1" time = "0.3" tokio = { version = "1", features = ["full"] } tokio-stream = "0.1" +tokio-util = "0.7" tonic = { version = "0.14", features = [ "gzip", "zstd", diff --git a/cross/Dockerfile.ent.musl b/cross/Dockerfile.ent.musl index 40bb8e2539..31da190ff8 100644 --- a/cross/Dockerfile.ent.musl +++ b/cross/Dockerfile.ent.musl @@ -11,6 +11,7 @@ RUN cp protoc/bin/protoc /usr/local/bin/ RUN cp -r protoc/include/google /usr/local/include/ COPY --from=ent ./ /home/runner/work/o2-enterprise/o2-enterprise/o2-enterprise +COPY --from=features ./ /home/runner/work/o2-enterprise/o2-enterprise/feature-definitions RUN test -n "${CROSS_SYSROOT:?CROSS_SYSROOT must be set by the base image}" && \ { test -d "${CROSS_SYSROOT%/}/usr/include" || \ { echo "Missing compiler include directory: ${CROSS_SYSROOT%/}/usr/include" >&2; exit 1; }; } && \ diff --git a/opencode.ci.jsonc b/opencode.ci.jsonc index 339d07aef0..b3a3dc64fa 100644 --- a/opencode.ci.jsonc +++ b/opencode.ci.jsonc @@ -1,7 +1,5 @@ { - // Loaded from the trusted base-branch checkout via OPENCODE_CONFIG when REVIEW_OPENCODE_CONFIG=ci-full. - // Same provider, model and agents as opencode.jsonc, but every tool (shell, edit, webfetch, ...) is allowed - // and nothing asks for permission. Keep the two files in step apart from the agent permissions. + // ci-full, loaded from the trusted base checkout: keep in step with opencode.jsonc except agent permissions (file tools, shell allowlist, no prompts). "$schema": "https://opencode.ai/config.json", // Nothing here may reach into the PR tree: no `instructions`, no `skills`, no plugins. @@ -28,8 +26,14 @@ "models": { "claude-sonnet-5-5": { "name": "Claude Sonnet 5.5", - // Selected with REVIEW_MODEL_VARIANT=high; opencode sends adaptive thinking and "output_config": { "effort": "high" }. - "variants": [{ "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }], + // REVIEW_MODEL_VARIANT and REVIEW_COORDINATOR_VARIANT pick one; opencode sends adaptive thinking and "output_config": { "effort": "" }. + "variants": [ + { "id": "low", "settings": { "thinking": { "type": "adaptive" }, "effort": "low" } }, + { "id": "medium", "settings": { "thinking": { "type": "adaptive" }, "effort": "medium" } }, + { "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }, + { "id": "xhigh", "settings": { "thinking": { "type": "adaptive" }, "effort": "xhigh" } }, + { "id": "max", "settings": { "thinking": { "type": "adaptive" }, "effort": "max" } } + ], "limit": { "context": 1000000, "output": 128000 } } } @@ -46,7 +50,7 @@ "models": { "deepseek-flash": { "name": "DeepSeek-V4.1-Flash", - // Selected with REVIEW_MODEL_VARIANT=max (the workflow matrix); opencode sends "reasoning_effort": "max". + // Selected with REVIEW_MODEL_VARIANT=max; opencode sends "reasoning_effort": "max". "variants": [{ "id": "max", "settings": { "reasoningEffort": "max" } }], "limit": { "context": 1048576, "output": 393216 } } @@ -56,52 +60,907 @@ "model": "anthropic-review/claude-sonnet-5-5", - // One allow-all rule: it comes after opencode's built-in defaults (last match wins), so no tool ever asks. + // Last match wins after opencode's built-in defaults, so nothing asks; the shell allowlist guards cost, not secrets (see README). "agents": { "ai-review-security": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-correctness": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-code-quality": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-performance": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-documentation": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-release": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-frontend": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-qa": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] }, "ai-review-coordinator": { "mode": "primary", "model": "anthropic-review/claude-sonnet-5-5", - "permissions": [{ "action": "*", "resource": "*", "effect": "allow" }] + "permissions": [ + { "action": "*", "resource": "*", "effect": "allow" }, + { "action": "webfetch", "resource": "*", "effect": "deny" }, + { "action": "websearch", "resource": "*", "effect": "deny" }, + { "action": "subagent", "resource": "*", "effect": "deny" }, + { "action": "question", "resource": "*", "effect": "deny" }, + { "action": "execute", "resource": "*", "effect": "deny" }, + { "action": "skill", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "*", "effect": "deny" }, + { "action": "shell", "resource": "git diff *", "effect": "allow" }, + { "action": "shell", "resource": "git log *", "effect": "allow" }, + { "action": "shell", "resource": "git show *", "effect": "allow" }, + { "action": "shell", "resource": "git blame *", "effect": "allow" }, + { "action": "shell", "resource": "git ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git status *", "effect": "allow" }, + { "action": "shell", "resource": "git rev-parse *", "effect": "allow" }, + { "action": "shell", "resource": "git merge-base *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -p *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -t *", "effect": "allow" }, + { "action": "shell", "resource": "git cat-file -s *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager diff *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager log *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager show *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager blame *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager ls-files *", "effect": "allow" }, + { "action": "shell", "resource": "git --no-pager status *", "effect": "allow" }, + { "action": "shell", "resource": "rg *", "effect": "allow" }, + { "action": "shell", "resource": "grep *", "effect": "allow" }, + { "action": "shell", "resource": "cat *", "effect": "allow" }, + { "action": "shell", "resource": "ls *", "effect": "allow" }, + { "action": "shell", "resource": "head *", "effect": "allow" }, + { "action": "shell", "resource": "tail *", "effect": "allow" }, + { "action": "shell", "resource": "find *", "effect": "allow" }, + { "action": "shell", "resource": "sed -n *", "effect": "allow" }, + { "action": "shell", "resource": "wc *", "effect": "allow" }, + { "action": "shell", "resource": "tail -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -f *", "effect": "deny" }, + { "action": "shell", "resource": "tail -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail * -F *", "effect": "deny" }, + { "action": "shell", "resource": "tail --follow*", "effect": "deny" }, + { "action": "shell", "resource": "tail * --follow*", "effect": "deny" }, + { "action": "shell", "resource": "find -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find * -exec *", "effect": "deny" }, + { "action": "shell", "resource": "find -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -execdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find * -ok *", "effect": "deny" }, + { "action": "shell", "resource": "find -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find * -okdir *", "effect": "deny" }, + { "action": "shell", "resource": "find -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find * -delete *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprint0 *", "effect": "deny" }, + { "action": "shell", "resource": "find -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fprintf *", "effect": "deny" }, + { "action": "shell", "resource": "find -fls *", "effect": "deny" }, + { "action": "shell", "resource": "find * -fls *", "effect": "deny" }, + { "action": "shell", "resource": "cat /*", "effect": "deny" }, + { "action": "shell", "resource": "cat * /*", "effect": "deny" }, + { "action": "shell", "resource": "cat *../*", "effect": "deny" }, + { "action": "shell", "resource": "cat *=/*", "effect": "deny" }, + { "action": "shell", "resource": "cat *environ*", "effect": "deny" }, + { "action": "shell", "resource": "head /*", "effect": "deny" }, + { "action": "shell", "resource": "head * /*", "effect": "deny" }, + { "action": "shell", "resource": "head *../*", "effect": "deny" }, + { "action": "shell", "resource": "head *=/*", "effect": "deny" }, + { "action": "shell", "resource": "head *environ*", "effect": "deny" }, + { "action": "shell", "resource": "tail /*", "effect": "deny" }, + { "action": "shell", "resource": "tail * /*", "effect": "deny" }, + { "action": "shell", "resource": "tail *../*", "effect": "deny" }, + { "action": "shell", "resource": "tail *=/*", "effect": "deny" }, + { "action": "shell", "resource": "tail *environ*", "effect": "deny" }, + { "action": "shell", "resource": "grep /*", "effect": "deny" }, + { "action": "shell", "resource": "grep * /*", "effect": "deny" }, + { "action": "shell", "resource": "grep *../*", "effect": "deny" }, + { "action": "shell", "resource": "grep *=/*", "effect": "deny" }, + { "action": "shell", "resource": "grep *environ*", "effect": "deny" }, + { "action": "shell", "resource": "rg /*", "effect": "deny" }, + { "action": "shell", "resource": "rg * /*", "effect": "deny" }, + { "action": "shell", "resource": "rg *../*", "effect": "deny" }, + { "action": "shell", "resource": "rg *=/*", "effect": "deny" }, + { "action": "shell", "resource": "rg *environ*", "effect": "deny" }, + { "action": "shell", "resource": "sed /*", "effect": "deny" }, + { "action": "shell", "resource": "sed * /*", "effect": "deny" }, + { "action": "shell", "resource": "sed *../*", "effect": "deny" }, + { "action": "shell", "resource": "sed *=/*", "effect": "deny" }, + { "action": "shell", "resource": "sed *environ*", "effect": "deny" }, + { "action": "shell", "resource": "wc /*", "effect": "deny" }, + { "action": "shell", "resource": "wc * /*", "effect": "deny" }, + { "action": "shell", "resource": "wc *../*", "effect": "deny" }, + { "action": "shell", "resource": "wc *=/*", "effect": "deny" }, + { "action": "shell", "resource": "wc *environ*", "effect": "deny" }, + { "action": "shell", "resource": "*>*", "effect": "deny" } + ] } } } diff --git a/opencode.jsonc b/opencode.jsonc index 18fef5e1a1..daf03d4660 100644 --- a/opencode.jsonc +++ b/opencode.jsonc @@ -27,8 +27,14 @@ "models": { "claude-sonnet-5-5": { "name": "Claude Sonnet 5.5", - // Selected with REVIEW_MODEL_VARIANT=high; opencode sends adaptive thinking and "output_config": { "effort": "high" }. - "variants": [{ "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }], + // REVIEW_MODEL_VARIANT and REVIEW_COORDINATOR_VARIANT pick one; opencode sends adaptive thinking and "output_config": { "effort": "" }. + "variants": [ + { "id": "low", "settings": { "thinking": { "type": "adaptive" }, "effort": "low" } }, + { "id": "medium", "settings": { "thinking": { "type": "adaptive" }, "effort": "medium" } }, + { "id": "high", "settings": { "thinking": { "type": "adaptive" }, "effort": "high" } }, + { "id": "xhigh", "settings": { "thinking": { "type": "adaptive" }, "effort": "xhigh" } }, + { "id": "max", "settings": { "thinking": { "type": "adaptive" }, "effort": "max" } } + ], "limit": { "context": 1000000, "output": 128000 } } } @@ -45,7 +51,7 @@ "models": { "deepseek-flash": { "name": "DeepSeek-V4.1-Flash", - // Selected with REVIEW_MODEL_VARIANT=max (the workflow matrix); opencode sends "reasoning_effort": "max". + // Selected with REVIEW_MODEL_VARIANT=max; opencode sends "reasoning_effort": "max". "variants": [{ "id": "max", "settings": { "reasoningEffort": "max" } }], "limit": { "context": 1048576, "output": 393216 } } diff --git a/scripts/ai-review/README.md b/scripts/ai-review/README.md index 52c78f05ee..b1b3bbb36d 100644 --- a/scripts/ai-review/README.md +++ b/scripts/ai-review/README.md @@ -1,7 +1,7 @@ # AI Code Review (`scripts/ai-review`) Automated PR review that posts a single consolidated comment on every pull request. Driven by -`.github/workflows/ai-code-review.yml` → `run-review.mjs`, using Claude Sonnet 5.5 (`anthropic-review/claude-sonnet-5-5`) at high effort with adaptive thinking via `opencode`. +`.github/workflows/ai-code-review.yml` → `run-review.mjs`, using Claude Sonnet 5.5 (`anthropic-review/claude-sonnet-5-5`) with adaptive thinking via `opencode`, at `high` effort unless the `AI_REVIEW_EFFORT` variable says otherwise (see Effort below). The posted comment is branded **OpenObserve Code Reviewer**. Branding is *cosmetic + identity only* — it does not change what the reviewer finds or decides. @@ -18,12 +18,12 @@ whose marker still names the provider, such as o2-enterprise's older one, keeps ## How it works (one paragraph) On each PR the workflow fetches the diff, filters noise, picks a risk tier (`trivial` / `lite` / -`full`), runs the tier's specialist agents (security, correctness, code-quality, performance, +`full` / `test-only` / `ci-only`), runs the tier's specialist agents (security, correctness, code-quality, performance, documentation) in parallel — plus any *conditional* agent whose domain the diff touches (frontend, qa, and release for migrations, `Cargo.toml`/`package.json`, `src/config/src/config.rs`, `.github/`, `deploy/` and Docker files) — and a coordinator consolidates their findings into one comment with a decision. Correctness owns the PR's behaviour end to end (callers, sibling -paths, state, the tests that prove the change); every tier runs it. Before the agents start, the +paths, state, the tests that prove the change); every tier but `ci-only` runs it. Before the agents start, the runner greps the PR tree for where each symbol the diff changes is used outside the diff, and for other definitions of the same name (`src/impact.mjs`), and gives that map to every reviewer. Security and correctness also get `openobserve-checklist.md`, the defect classes that recur in @@ -36,10 +36,54 @@ so one pass's misses can be another's catches (backtesting on an earlier model s findings two catch). The variable `AI_REVIEW_COLD_PASSES` sets the count (default `2`, clamped to 1–3; `src/passes.mjs`). The coordinator gets every pass's report labelled `, pass N/M` and keeps a finding raised by any pass, merging duplicates; the fallback comment unions the passes -and folds findings at the same file and nearby line when one title's words contain the other's (two P0/P1 blockers fold only on the same title). An area counts as +and folds findings at the same file and nearby line when one title's words contain the other's (two P0/P1 findings fold only on the same title). An area counts as reviewed when any of its passes finished, and the comment notes a lost pass. Each pass is billed, so the review costs about M times one pass; reviewer spans carry `review.pass`. +### Risk tiers + +`assessRiskTier` in `run-review.mjs` picks the tier. More than 50 files, or any path that looks +security-sensitive (`auth`, `token`, `secret`, `crypto`, `password`, `oauth`, `unsafe`, `acme`, +`cert`), is `full`. Otherwise a PR whose every file is test code (`tests/`, `test/`, `e2e/`, +`fixtures/`, `*.spec.*`, `*.test.*`, `*_test.rs`; `isTestOnlyPath` in `src/selection.mjs`) is +`test-only` and runs correctness and code-quality, whatever its size (qa joins when Playwright files change); a rename counts by both its source and +its destination, so code moved from `src/` into `tests/` is not test-only. A PR whose every file is CI +definition (`.github/workflows/**`, `.github/actions/**` or a top-level `.github/*.yml`; +`isCiOnlyPath`) is `ci-only` and runs security and release. Scripts, including this engine and the +two opencode configs, are code and take the size tiers below. Everything else is `trivial` (at most 10 +lines), `lite` (at most 100 lines) or `full`. Conditional agents are added on top as before, and +`force_full` still forces `full`. + +### Effort + +`AI_REVIEW_EFFORT` (`low`, `medium`, `high`, `xhigh` or `max`, default `high`) sets the reviewers' effort, and +`AI_REVIEW_COORDINATOR_EFFORT` the coordinator's (default: the reviewers' value). The workflow +passes them as `REVIEW_MODEL_VARIANT` and `REVIEW_COORDINATOR_VARIANT`, and the matrix carries no +variant of its own, so the variables are the only source. Each value is a variant of +`claude-sonnet-5-5` in both opencode configs, which opencode sends as adaptive thinking plus +`"output_config": { "effort": "" }`. A value that is not a declared variant stops the run +before it claims a pass or calls the model, and the comment's earlier verdict and `Auto-approved` +line give way to a 🔴 Low line saying the review did not run. Spans carry the effort in +`gen_ai.request.reasoning_effort`. + +### PRs that are never reviewed + +The workflow's `Skip PRs with nothing to review` step runs right after the engine checkout and +before anything is installed. It lists the PR's files (a rename lists its old path too) and the +author login, committer login and signature state of each commit, and runs `skip-check.mjs` (`nothingToReviewReason` in +`src/selection.mjs`). It skips nothing unless both listings match the PR's `changed_files` and +`commits` counts and stay under the endpoints' caps (3000 files, 250 commits; `listingComplete`), +since a listing cut short could hide a code file or a human commit. A PR is skipped, with nothing posted and no model called, when its author is +`dependabot[bot]` or `renovate[bot]` and every commit is authored by that same bot, committed by +it or by `web-flow` (how GitHub commits a bot's API-made commits) and carries a signature GitHub +verified, when every file is docs (`*.md` or `docs/**`, but not markdown under `scripts/ai-review/` or +`.github/`, nor any instruction loader the sanitizer strips, at any depth and in any case: `AGENTS.md`, `CLAUDE.md`, +`CONTEXT.md` or a file under `.claude/`, `.agents/` or `.opencode/`; `isLoaderPath` in `src/sanitize.mjs`), or when every file is one the +translation workflow commits (`web/src/locales/languages/*.json` other than `en-US.json`, which a +person writes, and `scripts/translations/.translation_state.json`). Every other bot or App author +is reviewed, and so is a dependency-bot PR someone else pushed to. A manual dispatch always +reviews, and so does a run where the file or commit listing, or `skip-check.mjs` itself, fails. + The coordinator also lists the findings it kept in a hidden JSON block, which the runner strips from the comment. Kept P0, P1 and P2 findings whose `file:line` is an added or context line of the diff are posted as one `COMMENT` review with inline threads (at most 10, never approve or request @@ -47,7 +91,7 @@ changes). Each thread carries a hidden fingerprint of file and title, so a re-ru already posted by `REVIEW_BOT_LOGIN`. A failed inline post only logs a warning. The threads are off by default; set the variable `AI_REVIEW_INLINE` to `true` to turn them on. -Findings are graded `p0` to `p3`. P0 and P1 are blockers and give the decision `blocked`; P2 and P3 +Findings are graded `p0` to `p3`. P0 (blocker) and P1 (major concern) both give the decision `blocked`; P2 and P3 do not block, and only those give `approved_with_comments`; no findings gives `approved`. A review keeps at most 3 P2 and 2 P3 findings: the coordinator is told to drop the rest, and the runner enforces the cap itself only in the fallback comment it renders when the coordinator fails. Legacy @@ -58,13 +102,34 @@ latest review is recorded, so a force-push back to an older reviewed head is rev finished review records what it covered in a hidden `o2-review-sha` tag (the head SHA, plus the paired head in a paired review, so a new commit on either side is new) and shows it in a visible `Reviewed commit` line with the review number. A run whose head matches the tag skips before any model call, which -covers a queued run, a re-label and a plain dispatch, including the paired dispatch from -openobserve. The tag is only written when it is true: the diff the reviewers read is rebuilt locally +covers a queued run, a re-label and the paired dispatch from openobserve (o2-enterprise does not +set `REVIEW_MANUAL_RUN`); a manual dispatch here, `force_full` or `ignore_pass_limit` reviews it again. The tag is only written when it is true: the diff the reviewers read is rebuilt locally from `MERGE_BASE_SHA` and `HEAD_SHA` (both sides of a paired review), because the API serves the branch as it is now. If that cannot be done, or no reviewer finished (each one failed or returned nothing parseable), the run writes no tag and names no commit, so that commit can be reviewed again. A run where no reviewer finished also reports `Not reviewed` and fails the check. +A stamped review that was complete (every reviewer pass finished without a failure, timeout or +wrap-up, and the coordinator merged them without falling back; `reviewComplete` in +`src/passes.mjs`) also records a fingerprint of the unfiltered diff, in a hidden +`o2-review-diff` tag (`diffFingerprint` in `src/pr-diff.mjs`: a SHA-256 of the diff with blob ids +and hunk line numbers removed but each hunk's function context kept, so the same lines changed in another function differ, while a rebase or a merge from main that leaves the PR's own changed +and context lines alone keeps it). Lockfiles, assets and generated files the reviewers never see are +in it, so adding one reviews again instead of carrying the old merge confidence over. Equal text proves +the same change only where it sits once in the base, so both the write and the skip also need every +hunk's old lines (context and removed lines) to occur exactly once in its base file (`hunksLocatable`); +an edit to one of two identical blocks, or a base file that cannot be read, reviews. When a new head's pinned diff has the same fingerprint, the run +skips with `unchanged_diff` before any model call: it uses no pass, keeps the review and its merge +confidence, and moves the `o2-review-sha` tag to the new head; the `Reviewed commit` line keeps the +commit the findings' line numbers refer to. Binary files keep their blob ids and trailing spaces count, so +both still change the fingerprint. The +check runs ahead of the review cap, so such a push is skipped, not refused, after the cap. A manual +dispatch (`REVIEW_MANUAL_RUN`), `force_full` or `ignore_pass_limit` always reviews, and so does a +diff that is not pinned to the head. In `approve` mode the skip still dismisses the earlier +approval, and the kept review's `Auto-approved` line becomes `Needs human review`, because a +skip does not rerun the approval gate's checks. A partial review records no fingerprint, so the next push +reviews again even when its diff is the same. + There is no cap on reviews per PR unless the variable `AI_REVIEW_MAX_PASSES` is a positive integer; unset, empty, `0` or an invalid value means no cap. Reviews are counted either way, in a hidden `o2-review-pass` tag written before the reviewers start, so a run stopped by removing the label or @@ -76,9 +141,9 @@ the list call fails) the run fails instead of reviewing, since neither the revie count can be checked. A push does not cancel a review in progress: the paid run finishes and only the newest push waits -in the concurrency queue. To review a commit again, dispatch the workflow with `ignore_pass_limit` -(it bypasses both the dedupe and the cap, and is forwarded to a paired o2-enterprise review) or with -`force_full` (it bypasses the dedupe only), or delete the review comment to reset the count. +in the concurrency queue. To review a commit again, dispatch the workflow (a plain dispatch or +`force_full` bypasses the dedupe only; `ignore_pass_limit` bypasses both the dedupe and the cap; the +two inputs are forwarded to a paired o2-enterprise review), or delete the review comment to reset the count. The comment marker is matched as a pattern, so a retired provider leg's comment is adopted and rewritten in place, including older comments whose marker was redacted to @@ -204,8 +269,8 @@ A PR must never be able to run code next to the review secrets or weaken its own | `src/opencode.mjs` env allowlist | `opencode serve` runs in `./pr` with only `PATH`, a fresh temp `HOME`/XDG dirs, `OPENCODE_CONFIG` (this repo's `opencode.jsonc` or `opencode.ci.jsonc` from `./engine`), the `OPENCODE_*` disable flags, a random server password and the one provider key. No `GH_*`, `GITHUB_*`, `ACTIONS_*`, `RUNNER_*` or `O2_*` variables. | | Config selection | `REVIEW_OPENCODE_CONFIG` = `default` (`opencode.jsonc`) or `ci-full` (`opencode.ci.jsonc`); any other value stops the run. The workflow sets `ci-full`, which keeps `approve` ineligible (see Auto-approve). | | `opencode.jsonc` permissions | Each `ai-review-*` agent: deny `*`, then allow `read`, `glob`, `grep`; `external_directory` denied. A `provider.use` policy allows only `anthropic-review`. LSP, formatters, snapshots and sharing are off. | -| `opencode.ci.jsonc` permissions | Same provider, model and agents, but each agent allows `*`: shell, edit, write and webfetch run with no prompt. The PR checkout is untrusted input to an agent that can run commands, and the provider key is in the opencode process environment, so a shell command can read it. | -| Startup self-check | Refuses to review if any agent is missing, any non-built-in plugin or MCP server loaded, or a granted tool's plugin is not active. Under `default` it also refuses any agent that resolves to allow a tool other than `read`/`glob`/`grep` or any outside directory; under `ci-full` it refuses an agent that does not allow every tool without asking. Permission prompts are rejected under `default` and approved once under `ci-full`. | +| `opencode.ci.jsonc` permissions | Same provider, model, variants and agents. Each agent allows `*` (read, glob, grep, edit, write run with no prompt), then denies `webfetch`, `websearch`, `subagent`, `question` (which would wait on an answer nobody in CI gives), `execute` (opencode 2.2.0's Code Mode: it runs JavaScript with `fetch`, so it would reach the network past the `webfetch` deny) and `skill` (reviewers read the skill copies with `read`). `shell` is kept to an allowlist: `git diff`, `git log`, `git show`, `git blame`, `git ls-files`, `git status` (each also after `git --no-pager`), `git rev-parse`, `git merge-base`, `git cat-file -p`/`-t`/`-s`, `rg`, `grep`, `cat`, `ls`, `head`, `tail`, `find`, `sed -n` and `wc`. Later deny rules refuse `find` with `-exec`, `-execdir`, `-ok`, `-okdir`, `-delete`, `-fprint`, `-fprint0`, `-fprintf` or `-fls` as a separate word; `tail -f`, `-F` and `--follow`; any absolute path, `../` path, `=/` argument or `environ` in `cat`, `head`, `tail`, `grep`, `rg`, `sed` and `wc` (so `/proc/*/environ` and `/dev/*` are not read through them); and any command containing `>` (an output redirect). `git grep` is not allowed (its `-O` runs a pager command). opencode checks every command in a pipeline, `&&` or `;` chain and inside `$(...)` or backticks, so the command runs only when every part is allowed, and a denied command fails outright, without a prompt. The allowlist stops builds, installs and long exploration. It is not a sandbox: `rg --pre` or a GNU `sed` `e` command can still run a program, a glob or a relative path inside the checkout is not inspected, and edit/write stay allowed in the disposable checkout. The PR checkout is untrusted input, and the provider key is in the opencode process environment, so a command that gets past these rules can read it. | +| Startup self-check | Refuses to review if any agent is missing, any non-built-in plugin or MCP server loaded, or a granted tool's plugin is not active. Under `default` it also refuses any agent that resolves to allow a tool other than `read`/`glob`/`grep` or any outside directory; under `ci-full` it refuses an agent that does not allow read, glob, grep, edit and write without asking, that leaves webfetch, websearch, subagent, question, execute or skill enabled, or whose shell is denied or not restricted to an allowlist. Permission prompts are rejected under `default` and approved once under `ci-full`. | | `src/comments.mjs` | The previous review is only ever a marker comment by the reviewer app (`REVIEW_BOT_LOGIN` = `[bot]`), read across every page; anyone else's comment carrying the marker is never fed to the reviewers or edited. When the app token cannot be minted the review posts as `github-actions[bot]`, which any same-repo PR workflow can also post as, so that run uses no previous review and posts a fresh comment. | | `src/scrub.mjs` | Before posting: redacts values of secret-named env vars and token-shaped strings (inside code too), breaks forged `"; @@ -6,6 +6,8 @@ export const REVIEW_PASS_TAG = ""; export const REVIEW_SHA_TAG = ""; // The verdict a later head's line replaced, kept in case the head returns to the reviewed commit. export const REVIEW_HELD_TAG = ""; +// Fingerprint of the diff the sha tag's review read, so a later head with the same changes (rebase, merge from main) needs no new review. +export const REVIEW_DIFF_TAG = ""; // Matched as a pattern so a retired leg's comment is still adopted; `[redacted]` is what redaction left in comments whose marker named the provider. export const ANY_REVIEW_MARKER = ""; // Tags a comment that stands in for a review (post-paired-pointer.sh, the "key not set" notice, the in-progress claim); it is replaced in place but is not a previous review. @@ -26,7 +28,7 @@ const AUTO_APPROVE_LINE_RE = /^(?:[^\w\n`]|<\/?[a-z]+>)*(?:auto[- ]?approved|nee const FORGED_LINE_RE = /mergeconfidence|autoapproved|needshumanreview|reviewedcommit/; // The fixed head of a finding line, glyph and location then category, which leaves no room for a status phrase ahead of the category. const FINDING_LINE_RE = /^\s*- (?:[^\w\s`*<]+\s+)?(?:`[^`\n]+`\s+)?\*\*\[[^\]\n]+\]\*\*/; -const TRAILING_TAGS_RE = /(?:\s*)+\s*$/i; +const TRAILING_TAGS_RE = /(?:\s*)+\s*$/i; const HTML_COMMENT_RE = /` : bare; +} + +// A forced run (dispatch, force_full, ignore_pass_limit) always reviews; otherwise a pinned diff equal to the reviewed one is skipped. +export function shouldSkipAsUnchangedDiff(body, fingerprint, { pinned = false, force = false } = {}) { + if (force || !pinned || !fingerprint) return false; + return reviewedDiffFingerprint(body) === fingerprint; +} + +// Costs no pass; the visible commit stays the reviewed one because finding line numbers refer to it, while the tag moves so the next push compares against this head. +export function bodyAtUnchangedDiff(body, key, maxPasses) { + return withReviewShaTag(bodyAtReviewedHead(body, maxPasses), key); +} + +export function shouldSkipAsReviewed(body, key, { ignorePassLimit = false, forceFull = false, manual = false } = {}) { + if (!key || ignorePassLimit || forceFull || manual) return false; return reviewedKey(body) === key; } @@ -103,8 +127,8 @@ export function passLimitReached(passes, maxPasses) { } // Dedupe comes first so a queued run on a reviewed head stays silent instead of rewriting the limit note. -export function reviewSkipReason({ body, key, passesDone, maxPasses, ignorePassLimit = false, forceFull = false }) { - if (shouldSkipAsReviewed(body, key, { ignorePassLimit, forceFull })) return "already_reviewed"; +export function reviewSkipReason({ body, key, passesDone, maxPasses, ignorePassLimit = false, forceFull = false, manual = false }) { + if (shouldSkipAsReviewed(body, key, { ignorePassLimit, forceFull, manual })) return "already_reviewed"; if (!ignorePassLimit && passLimitReached(passesDone, maxPasses)) return "pass_limit_reached"; return ""; } @@ -207,20 +231,25 @@ export function bodyAtPassLimit(body, maxPasses, head) { return bodyAtUnreviewedHead(withPassLimitNote(body, passLimitNote(maxPasses, reviewedKey(body), head)), head); } +// Even on the covered commit: the run already dismissed the approval, so the old verdict and gate line must not read as current. +export function bodyAtFailedRun(body, head) { + return head ? withConfidenceLine(withHeldVerdict(body), notRunConfidenceLine(shortKey(head))) : body; +} + // The sha tag says that exact commit was read and stops it being reviewed again, so a run that cannot prove that leaves it off. -export function finishReviewBody(body, { key, passes, maxPasses, reviewed, confidence, gateLine = "" }) { +export function finishReviewBody(body, { key, passes, maxPasses, reviewed, confidence, gateLine = "", diffFingerprint = "" }) { const stamped = reviewed && key; const lines = [confidence ? mergeConfidenceLine(confidence) : "", gateLine].filter(Boolean); let out = insertAfterCallout(body, lines.join("\n\n")); if (stamped) out += `\n\n${reviewedCommitLine(key, passes, maxPasses)}`; if (passLimitReached(passes, maxPasses)) out += `\n\n${passLimitNote(maxPasses)}`; - if (stamped) out = withReviewShaTag(out, key); + if (stamped) out = withReviewDiffTag(withReviewShaTag(out, key), diffFingerprint); return withPassTag(out, passes); } // What the reviewers see as : the review alone, none of the runner's bookkeeping. export function previousReviewText(body) { - return [REVIEW_SHA_TAG, REVIEW_PASS_TAG, REVIEW_HELD_TAG].reduce(withoutTag, stripRunnerLines(String(body ?? ""))); + return [REVIEW_SHA_TAG, REVIEW_PASS_TAG, REVIEW_HELD_TAG, REVIEW_DIFF_TAG].reduce(withoutTag, stripRunnerLines(String(body ?? ""))); } // Yields the text after each pass, last the first that changed nothing: a removal can splice a new match out of what surrounds it. @@ -303,7 +332,7 @@ function unwrapJsonBody(body) { export function sanitizeReviewBody(body, { marker, providerId, modelId, env }) { if (!body) return { text: "", secretHits: 0 }; const anyMarker = new RegExp(ANY_REVIEW_MARKER, "gi"); - const runnerTags = new RegExp(`${ANY_REVIEW_MARKER}|${REVIEW_PASS_TAG}|${REVIEW_SHA_TAG}|${REVIEW_HELD_TAG}`, "gi"); + const runnerTags = new RegExp(`${ANY_REVIEW_MARKER}|${REVIEW_PASS_TAG}|${REVIEW_SHA_TAG}|${REVIEW_HELD_TAG}|${REVIEW_DIFF_TAG}`, "gi"); let cleaned = unwrapJsonBody(body) .replace(/\\n/g, "\n") diff --git a/scripts/ai-review/src/confidence.mjs b/scripts/ai-review/src/confidence.mjs index 9345a11008..428d814ee6 100644 --- a/scripts/ai-review/src/confidence.mjs +++ b/scripts/ai-review/src/confidence.mjs @@ -25,8 +25,13 @@ export function reviewerFinished(r) { return !r.skipped && !r.error && r.parsed === true; } +// A reviewer interrupted at its soft deadline answered in haste, so its output counts as findings but not as a finished review. +export function reviewerCompleted(r) { + return reviewerFinished(r) && !r.wrappedUp; +} + export function unfinishedReviewers(ranResults) { - return ranResults.filter(r => !reviewerFinished(r)).map(r => r.agentName); + return ranResults.filter(r => !reviewerCompleted(r)).map(r => r.agentName); } function plural(count, noun) { @@ -50,12 +55,14 @@ export function mergeConfidence({ if (ranCount === 0) return { level: "low", reasons: ["no reviewer ran"] }; if (ranCount > 0 && failedReviewers.length >= ranCount) return { level: "low", reasons: ["no reviewer finished"] }; - const blockers = (counts.p0 ?? 0) + (counts.p1 ?? 0); + const blockers = counts.p0 ?? 0; + const major = counts.p1 ?? 0; const core = failedReviewers.filter(name => CORE_REVIEWER_RE.test(shortName(name))); const other = failedReviewers.filter(name => !core.includes(name)); const unfinished = name => `${shortName(name)} reviewer did not finish`; const low = [ - ...(blockers > 0 ? [`${plural(blockers, "blocker")} (P0/P1)`] : []), + ...(blockers > 0 ? [`${plural(blockers, "blocker")} (P0)`] : []), + ...(major > 0 ? [`${plural(major, "major concern")} (P1)`] : []), ...(findingsKnown ? [] : ["findings could not be counted"]), ...core.map(unfinished), ]; @@ -78,6 +85,10 @@ export function staleConfidenceLine(shortHead) { return `**Merge confidence: ${CONFIDENCE_LABEL.low}** — \`${shortHead}\` has not been reviewed`; } +export function notRunConfidenceLine(shortHead) { + return `**Merge confidence: ${CONFIDENCE_LABEL.low}** — the review did not run because of a CI configuration error; \`${shortHead}\` has not been reviewed`; +} + export function reviewingConfidenceLine(shortHead) { return `**Merge confidence: 🟡 Reviewing** — \`${shortHead}\` is being reviewed`; } diff --git a/scripts/ai-review/src/jsonc.mjs b/scripts/ai-review/src/jsonc.mjs new file mode 100644 index 0000000000..3fd4bb70ad --- /dev/null +++ b/scripts/ai-review/src/jsonc.mjs @@ -0,0 +1,18 @@ +// Comments and trailing commas out, so the opencode configs parse as JSON. +export function stripJsonc(text) { + let out = ""; + for (let i = 0; i < text.length; i++) { + const c = text[i]; + if (c === '"') { + const start = i; + for (i++; i < text.length && text[i] !== '"'; i++) if (text[i] === "\\") i++; + out += text.slice(start, i + 1); + } else if (c === "/" && text[i + 1] === "/") { + while (i < text.length && text[i] !== "\n") i++; + out += "\n"; + } else { + out += c; + } + } + return out.replace(/,(\s*[}\]])/g, "$1"); +} diff --git a/scripts/ai-review/src/opencode.mjs b/scripts/ai-review/src/opencode.mjs index abe6174c79..6ecc0e7141 100644 --- a/scripts/ai-review/src/opencode.mjs +++ b/scripts/ai-review/src/opencode.mjs @@ -23,7 +23,11 @@ export const BUILTIN_TOOL_IDS = new Set([ const ALLOWED_TOOL_IDS = new Set(["read", "glob", "grep"]); const EDIT_TOOL_IDS = new Set(["edit", "write"]); // What ci-full must actually grant without asking; a config that fell back to the defaults fails here. -const CI_FULL_TOOL_IDS = ["read", "glob", "grep", "edit", "write", "shell", "webfetch"]; +const CI_FULL_TOOL_IDS = ["read", "glob", "grep", "edit", "write"]; +// question waits on an answer nobody in CI gives; execute runs JavaScript with fetch, so it would bypass the webfetch deny. +const CI_FULL_DENIED_TOOL_IDS = ["webfetch", "websearch", "subagent", "question", "execute", "skill"]; +// Offered, but only for the commands the config allowlists. +const CI_FULL_RESTRICTED_TOOL_IDS = ["shell"]; // 2.x dropped OPENCODE_PURE and the plugin, skill and LSP-download flags; sanitizePrTree covers what they did. const SANDBOX_FLAGS = { @@ -42,6 +46,7 @@ const INTERRUPTED_ERROR = { type: "interrupted", message: "opencode interrupted const DEFAULT_REQUEST_HEADERS_TIMEOUT_MS = 30_000; const STATUS_POLL_MS = 15_000; const STREAM_RETRY_MS = 1_000; +const INTERRUPT_SETTLE_MS = 20_000; const TRANSIENT_ERROR_RE = /ECONNRESET|ECONNREFUSED|ETIMEDOUT|EPIPE|socket hang up|overloaded|rate limit|too many requests|temporarily unavailable|bad gateway|service unavailable|gateway timeout/i; // opencode 2.x StructuredError types; the first set is worth a fresh session, the second never is, whatever the message says. const TRANSIENT_ERROR_TYPES = new Set(["provider.rate-limit", "provider.transport", "provider.timeout", "provider.internal"]); @@ -217,7 +222,7 @@ export class OpencodeClient { } // Sends the prompt (2.x admits it and returns at once) and waits on the event stream, so no request ever blocks for the length of an agent run (undici's 300 s headersTimeout killed every long review). - async prompt({ agent, model, variant, system, text, title, timeoutMs }) { + async prompt({ agent, model, variant, system, text, title, timeoutMs, wrapUp = null }) { await this.ensureEventStream(); // 2.x binds agent, model and variant to the session; the prompt itself carries only text. const session = (await this.json("POST", "/api/session", { @@ -233,17 +238,20 @@ export class OpencodeClient { if (system) { await this.json("PUT", `/api/experimental/session/${session.id}/instructions/entries/${SYSTEM_ENTRY_KEY}`, { value: system }); } + const softMs = wrapUpStartMs(timeoutMs, wrapUp); // Armed before the POST: the run can fail before the prompt call returns, and nothing would replay it. - const finished = waiter.wait(timeoutMs); + const finished = waiter.wait(softMs); await this.json("POST", `/api/session/${session.id}/prompt`, { text }); - const outcome = await finished; + let outcome = await finished; + let wrappedUp = false; + if (outcome.timedOut && softMs < timeoutMs) ({ outcome, wrappedUp } = await this.wrapUpSession(session.id, waiter, wrapUp.text, timeoutMs - softMs)); if (outcome.timedOut) { await this.request("POST", `/api/session/${session.id}/interrupt`).catch(() => {}); throw new OpencodeError(`opencode agent ${agent} timed out after ${timeoutMs}ms`, { name: "TimeoutError" }); } const messages = await this.json("GET", `/api/session/${session.id}/message?type=assistant&order=desc&limit=1`); totals = await this.json("GET", `/api/session/${session.id}`).catch(() => null); - return finalAssistantResult(messages?.data, totals?.data, outcome.error, waiter.rejectedPermissions); + return { ...finalAssistantResult(messages?.data, totals?.data, outcome.error, waiter.rejectedPermissions), wrappedUp }; } catch (err) { // A failed or timed-out session was still billed; its totals are gone once the session is deleted. totals ??= await this.json("GET", `/api/session/${session.id}`).catch(() => null); @@ -256,6 +264,23 @@ export class OpencodeClient { } } + // The interrupt keeps the finished steps in the session, so the follow-up prompt answers from everything the agent already read. + async wrapUpSession(sessionID, waiter, text, budgetMs) { + const deadline = Date.now() + budgetMs; + const stopped = waiter.wait(Math.min(INTERRUPT_SETTLE_MS, budgetMs)); + await this.request("POST", `/api/session/${sessionID}/interrupt`).catch(() => {}); + const stop = await stopped; + // An unconfirmed interrupt ends as the hard timeout; a run that ended on its own keeps its own outcome. + if (stop.timedOut || stop.error?.type !== "interrupted") return { outcome: stop, wrappedUp: false }; + // Nothing interrupts the follow-up before its wait ends, so a late interrupted event or idle record is the first run's. + waiter.ignoreInterrupted = true; + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) return { outcome: { timedOut: true, error: null }, wrappedUp: true }; + const finished = waiter.wait(remainingMs); + await this.json("POST", `/api/session/${sessionID}/prompt`, { text }); + return { outcome: await finished, wrappedUp: true }; + } + close() { this.closed = true; this.stream?.close(); @@ -270,6 +295,7 @@ class SessionWaiter { this.sessionID = sessionID; this.rejectedPermissions = 0; this.settle = null; + this.ignoreInterrupted = false; } wait(timeoutMs) { @@ -291,6 +317,7 @@ class SessionWaiter { } onError(error) { + if (this.ignoreInterrupted && error?.type === "interrupted") return; this.settle?.({ timedOut: false, error: error || { type: "unknown", message: "opencode run failed" } }); } @@ -353,11 +380,13 @@ function permissionAction(toolId) { return EDIT_TOOL_IDS.has(toolId) ? "edit" : toolId; } +// Same as opencode 2.2.0: a trailing " *" also matches the bare command, so "git diff *" covers "git diff". export function wildcardMatch(input, pattern) { - const escaped = pattern + let escaped = pattern .replace(/[.+^${}()|[\]\\]/g, "\\$&") .replace(/\*/g, ".*") .replace(/\?/g, "."); + if (escaped.endsWith(" .*")) escaped = `${escaped.slice(0, -3)}( .*)?`; return new RegExp(`^${escaped}$`, "s").test(input); } @@ -375,7 +404,7 @@ export async function selfCheck(client, expectedAgents, { readOnly = true } = {} const broken = plugins.filter(p => p.state?.status !== "active"); if (broken.length > 0) throw new OpencodeError(`opencode plugins failed to load: ${broken.map(p => `${p.id}: ${p.state?.error || "unknown"}`).join("; ")}`); const active = new Set(plugins.map(p => p.id)); - const missing = (readOnly ? [...ALLOWED_TOOL_IDS] : CI_FULL_TOOL_IDS).filter(id => !active.has(`opencode.tool.${id}`)); + const missing = (readOnly ? [...ALLOWED_TOOL_IDS] : [...CI_FULL_TOOL_IDS, ...CI_FULL_RESTRICTED_TOOL_IDS]).filter(id => !active.has(`opencode.tool.${id}`)); if (missing.length > 0) throw new OpencodeError(`opencode tools did not load: ${missing.join(", ")}`); const mcp = (await client.json("GET", "/api/mcp"))?.data || []; if (mcp.length > 0) throw new OpencodeError(`opencode loaded MCP servers; refusing to review`); @@ -397,9 +426,7 @@ export async function selfCheck(client, expectedAgents, { readOnly = true } = {} const problem = externalDirectoryProblem(ruleset); if (problem) problems.push(`${name}: ${problem}`); } else { - for (const toolId of CI_FULL_TOOL_IDS) { - if (!toolAllowedWithoutAsking(toolId, ruleset)) problems.push(`${name}: ${toolId} not fully allowed`); - } + problems.push(...ciFullProblems(ruleset).map(problem => `${name}: ${problem}`)); } } if (problems.length > 0) throw new OpencodeError(`opencode permission self-check failed: ${problems.join("; ")}`); @@ -415,6 +442,28 @@ export function externalDirectoryProblem(ruleset) { return extra.length > 0 ? `external_directory allows ${extra.map(r => r.resource).join(", ")}` : ""; } +// ci-full grants the file tools outright, denies the tools that only cost time, and keeps the shell on an allowlist. +export function ciFullProblems(ruleset) { + const problems = CI_FULL_TOOL_IDS.filter(id => !toolAllowedWithoutAsking(id, ruleset)).map(id => `${id} not fully allowed`); + problems.push(...CI_FULL_DENIED_TOOL_IDS.filter(id => toolEnabled(id, ruleset)).map(id => `${id} allowed`)); + for (const id of CI_FULL_RESTRICTED_TOOL_IDS) { + if (!toolEnabled(id, ruleset)) problems.push(`${id} denied`); + else if (toolAllowedWithoutAsking(id, ruleset)) problems.push(`${id} allows every command`); + } + return problems; +} + +// The follow-up only gets a slot when the first run keeps at least as long as the follow-up; shorter sessions run to the hard timeout. +export function wrapUpStartMs(timeoutMs, wrapUp) { + return wrapUp?.ms > 0 && wrapUp.text && timeoutMs >= 2 * wrapUp.ms ? timeoutMs - wrapUp.ms : timeoutMs; +} + +// The variant ids a model declares in an opencode config; a variant opencode does not know must fail the run before any session starts. +export function modelVariantIds(config, providerId, modelId) { + const variants = config?.providers?.[providerId]?.models?.[modelId]?.variants; + return Array.isArray(variants) ? variants.map(v => v?.id).filter(Boolean) : []; +} + // A retry must leave room for its own full timeout plus whatever still has to run after it (the coordinator). export function shouldRetry({ err, attempt, maxRetries, remainingMs, timeoutMs, reserveMs = 0 }) { return attempt <= maxRetries && isTransientError(err) && remainingMs > timeoutMs + reserveMs; diff --git a/scripts/ai-review/src/passes.mjs b/scripts/ai-review/src/passes.mjs index a262e13886..c0048a7039 100644 --- a/scripts/ai-review/src/passes.mjs +++ b/scripts/ai-review/src/passes.mjs @@ -1,4 +1,4 @@ -import { reviewerFinished, shortName } from "./confidence.mjs"; +import { reviewerCompleted, reviewerFinished, shortName } from "./confidence.mjs"; export const DEFAULT_COLD_PASSES = 2; export const MAX_COLD_PASSES = 3; @@ -43,8 +43,9 @@ export async function runPooled(items, limit, worker) { function mergeArea(group) { const finished = group.filter(reviewerFinished); + const completed = finished.filter(reviewerCompleted); const ran = group.filter(r => !r.skipped); - const base = finished[0] ?? ran[0] ?? group[0]; + const base = completed[0] ?? finished[0] ?? ran[0] ?? group[0]; const covered = finished.length > 0; return { agentKey: base.agentKey, @@ -58,6 +59,8 @@ function mergeArea(group) { skipped: group.every(r => r.skipped), fullDiff: base.fullDiff, parsed: covered, + // Only an uninterrupted pass completes an area; wrapped-up output still counts as findings. + wrappedUp: covered && completed.length === 0, passes: group.length, passesFinished: finished.length, passResults: group, @@ -78,6 +81,12 @@ export function lostPasses(results) { return results.filter(r => !r.skipped && r.passesFinished > 0 && r.passesFinished < r.passes); } +// Only a review with every pass finished in time and a real coordinator merge may let the next push with the same diff skip. +export function reviewComplete(passResults, coordinatorResult) { + if (!coordinatorResult || coordinatorResult.fallback) return false; + return passResults.every(r => r.skipped || reviewerCompleted(r)); +} + // Written by code like the failed-reviewers line, so a reader learns an area got less scrutiny than the rest. export function lostPassesNote(results) { const lost = lostPasses(results); diff --git a/scripts/ai-review/src/pr-diff.mjs b/scripts/ai-review/src/pr-diff.mjs index 0563222bf3..0359400737 100644 --- a/scripts/ai-review/src/pr-diff.mjs +++ b/scripts/ai-review/src/pr-diff.mjs @@ -1,4 +1,5 @@ import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; const MAX_BUFFER = 200 * 1024 * 1024; const TOO_LARGE_RE = /exceeded the maximum number of lines|too_large|HTTP 406|HTTP 422/i; @@ -55,6 +56,39 @@ export function fetchPinnedDiff(params) { return pinDiff({ ...params, diff: fetchDiff(params) }); } +// Blob ids and hunk line numbers move with the base, so a rebase keeps the fingerprint; the function context after `@@` stays, as the same lines in another function are another change. +export function diffFingerprint(diff) { + const text = String(diff ?? ""); + // A binary file has no hunks, so its blob ids are the only record of its content. + const keepBlobIds = /^(Binary files .* differ|GIT binary patch)$/m.test(text); + const normalized = text + .split("\n") + .map(line => (keepBlobIds ? line : line.replace(/^index [0-9a-f]+\.\.[0-9a-f]+/, "index"))) + .map(line => line.replace(/^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@/, "@@")) + .join("\n") + .replace(/\n+$/, ""); + return createHash("sha256").update(normalized).digest("hex"); +} + +// Equal text names one change only where each hunk's old lines sit once in the base, so an edit to one of two identical blocks cannot pass for the other. +export function hunksLocatable(diff, readBase) { + for (const section of String(diff ?? "").split(/^diff --git /m).slice(1)) { + const firstHunk = section.search(/^@@ /m); + if (firstHunk === -1) continue; + const header = section.slice(0, firstHunk); + if (/^--- \/dev\/null$/m.test(header)) continue; + const path = header.match(/^--- a\/(.*?)\t?$/m)?.[1]; + const base = path ? readBase(path) : null; + if (typeof base !== "string") return false; + const baseLines = base.split("\n"); + for (const hunk of section.slice(firstHunk).split(/^@@ [^\n]*(?:\n|$)/m).slice(1)) { + const old = hunk.split("\n").filter(line => line.startsWith(" ") || line.startsWith("-")).map(line => line.slice(1)); + if (old.length === 0 ? base !== "" : occurrences(baseLines, old) !== 1) return false; + } + } + return true; +} + // `unfocused` keeps the whole diff for a gate candidate, whose approval must rest on reviewers that saw every file; requiresFocus agents still opt out. export function reviewerDiff(diff, { fileFocus, requiresFocus }, { unfocused = false, pathOf = path => path } = {}) { if (!fileFocus) return { diff, skipped: false, fullDiff: true }; @@ -107,3 +141,12 @@ function prefixGitHeader(line, p) { function prefixBinaryLine(line, p) { return line.replace(/(^Binary files |and )("?)([ab])\//g, `$1$2$3/${p}/`); } + +// Stops at two: the caller only needs to know whether a block is unique. +function occurrences(lines, block) { + let found = 0; + for (let i = 0; i + block.length <= lines.length && found < 2; i += 1) { + if (lines[i] === block[0] && block.every((line, j) => lines[i + j] === line)) found += 1; + } + return found; +} diff --git a/scripts/ai-review/src/report.mjs b/scripts/ai-review/src/report.mjs index e188203c07..3b0572a462 100644 --- a/scripts/ai-review/src/report.mjs +++ b/scripts/ai-review/src/report.mjs @@ -28,7 +28,7 @@ const BLOCKQUOTE_LINE_RE = /^[ \t]*>.*$/gm; const SEVERITY_GLYPH = { p0: "🔴", p1: "🟠", p2: "🟡", p3: "🔵" }; const SEVERITY_SECTION = { p0: "#### 🔴 P0 · Blockers", - p1: "#### 🟠 P1 · Blockers", + p1: "#### 🟠 P1 · Major concerns", p2: "#### 🟡 P2 · Should fix", p3: "#### 🔵 P3 · Worth a look", }; @@ -280,7 +280,8 @@ ${failedLine} const bySeverity = groupBySeverity(shown); const sections = severitySections(bySeverity); const blocked = bySeverity.p0.length + bySeverity.p1.length > 0; - const callout = blocked ? "> [!CAUTION]\n> ### ⛔ Blockers found" : "> [!NOTE]\n> ### 💬 Approved with comments"; + const heading = bySeverity.p0.length > 0 ? "⛔ Blockers found" : "⚠️ Major concerns found"; + const callout = blocked ? `> [!CAUTION]\n> ### ${heading}` : "> [!NOTE]\n> ### 💬 Approved with comments"; const rawCount = agentResults.flatMap(r => r.findings).length; const dedupNote = fallbackDedupNote(rawCount, dedupeFindings(agentResults).length, shown.length); diff --git a/scripts/ai-review/src/sanitize.mjs b/scripts/ai-review/src/sanitize.mjs index fc9ad5666f..4357a57629 100644 --- a/scripts/ai-review/src/sanitize.mjs +++ b/scripts/ai-review/src/sanitize.mjs @@ -38,6 +38,12 @@ export function sanitizePrTree(prDir, contextSourceDir) { return { removed, contextFiles: copied }; } +// The same names the walk strips, so a PR touching only these still steers a reviewer and is never docs-only. +export function isLoaderPath(filePath) { + const parts = String(filePath ?? "").toLowerCase().split("/"); + return LOADER_FILES.has(parts.at(-1)) || parts.some(part => LOADER_DIRS.has(part)); +} + function walk(dir, root, removed) { for (const entry of readdirSync(dir, { withFileTypes: true })) { const path = join(dir, entry.name); diff --git a/scripts/ai-review/src/selection.mjs b/scripts/ai-review/src/selection.mjs index 1ddd11c8c5..9bc977444a 100644 --- a/scripts/ai-review/src/selection.mjs +++ b/scripts/ai-review/src/selection.mjs @@ -1,5 +1,12 @@ import { existsSync } from "node:fs"; import { basename, dirname, join } from "node:path"; +import { isLoaderPath } from "./sanitize.mjs"; + +// Dependency bots whose PRs only bump versions; any other bot or App can write code and is reviewed. +const DEPENDENCY_BOTS = new Set(["dependabot[bot]", "renovate[bot]"]); +// GitHub's files and commits endpoints stop at these counts however far they are paged. +const MAX_LISTED_FILES = 3000; +const MAX_LISTED_COMMITS = 250; // Diffs that can affect a release: migrations, dependency manifests, server config, CI and deployment files. export function isReleaseFile(filePath) { @@ -22,3 +29,57 @@ export function resolveRulesFiles(files, exists = existsSync) { } return out; } + +// Test code only: test trees, spec/test files and fixtures, in any language. +export function isTestOnlyPath(filePath) { + if (isCiOnlyPath(filePath)) return false; + if (/(^|\/)(tests?|__tests__|e2e|fixtures?|testdata)\//.test(filePath)) return true; + if (/\.(spec|test)\.[cm]?[jt]sx?$/.test(filePath)) return true; + return /(^|\/)[^/]+_test\.(rs|go|py)$/.test(filePath) || /(^|\/)test_[^/]+\.py$/.test(filePath); +} + +// Workflows, actions and .github config only; scripts, the review engine and its opencode configs are code and take the normal tiers. +export function isCiOnlyPath(filePath) { + return /^\.github\/((workflows|actions)\/|[^/]+\.ya?ml$)/.test(filePath); +} + +export function narrowTier(paths) { + if (paths.length === 0) return ""; + if (paths.every(isTestOnlyPath)) return "test-only"; + if (paths.every(isCiOnlyPath)) return "ci-only"; + return ""; +} + +// Prompt, agent-rule and workflow markdown steers the AI or CI, so it is reviewed like code. +export function isDocsPath(filePath) { + if (/^(scripts\/ai-review\/|\.github\/)/.test(filePath) || isLoaderPath(filePath)) return false; + return /\.md$/i.test(filePath) || /^docs\//.test(filePath); +} + +// What update-translations.yml commits; en-US.json is the English source a person writes, so it is still reviewed. +export function isTranslationPath(filePath) { + if (filePath === "scripts/translations/.translation_state.json") return true; + return /^web\/src\/locales\/languages\/[^/]+\.json$/.test(filePath) && !filePath.endsWith("/en-US.json"); +} + +// A listing is whole only when it matches the PR's own counts below each endpoint's cap; anything else may hide a file or commit. +export function listingComplete({ files, commits, expectedFiles, expectedCommits }) { + const count = value => (/^\d+$/.test(String(value ?? "").trim()) ? Number(value) : Number.NaN); + const wantFiles = count(expectedFiles); + const wantCommits = count(expectedCommits); + return files === wantFiles && commits === wantCommits && wantFiles < MAX_LISTED_FILES && wantCommits < MAX_LISTED_COMMITS; +} + +// GitHub commits a bot's API-made commits as web-flow, so the committer is either the bot or web-flow, and GitHub must have verified the signature. +export function botCommit(commit, bot) { + return commit?.author === bot && (commit.committer === bot || commit.committer === "web-flow") && commit.verified === true; +} + +// Why a PR needs no review at all, or "" when it does; a PR with no listed files is reviewed so a failed listing never skips one. +export function nothingToReviewReason({ author = "", commits = [], paths = [] }) { + if (DEPENDENCY_BOTS.has(author) && commits.length > 0 && commits.every(c => botCommit(c, author))) return "bot_author"; + if (paths.length === 0) return ""; + if (paths.every(isDocsPath)) return "docs_only"; + if (paths.every(isTranslationPath)) return "translation_only"; + return ""; +} diff --git a/scripts/ai-review/src/severity.mjs b/scripts/ai-review/src/severity.mjs index cd872c527e..33ba5be12a 100644 --- a/scripts/ai-review/src/severity.mjs +++ b/scripts/ai-review/src/severity.mjs @@ -1,5 +1,5 @@ export const SEVERITIES = ["p0", "p1", "p2", "p3"]; -// Per-review limits; p0 and p1 are never capped because a blocker must always be reported. +// Per-review limits; p0 and p1 are never capped because a finding that blocks must always be reported. export const SEVERITY_CAPS = { p2: 3, p3: 2 }; const LEGACY_SEVERITIES = new Map([ ["critical", "p0"], diff --git a/scripts/ai-review/test/comments.test.mjs b/scripts/ai-review/test/comments.test.mjs index 3b5e3bd080..0da4a7e729 100644 --- a/scripts/ai-review/test/comments.test.mjs +++ b/scripts/ai-review/test/comments.test.mjs @@ -1,12 +1,14 @@ import test from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; import { ANY_REVIEW_MARKER, REVIEW_HEADING, bodyAtPassLimit, bodyAtReviewedHead, + bodyAtUnchangedDiff, bodyAtUnreviewedHead, claimedReviewBody, finishReviewBody, @@ -20,16 +22,18 @@ import { reviewShaTag, reviewSkipReason, reviewedCommitLine, + reviewedDiffFingerprint, reviewedKey, sanitizeReviewBody, selectOwnReviewComment, shouldSkipAsReviewed, + shouldSkipAsUnchangedDiff, withPassLimitNote, withPassTag, withReviewShaTag, withoutAutoApproveLine, } from "../src/comments.mjs"; -import { autoApproveLine } from "../src/auto-approve.mjs"; +import { autoApproveLine, withoutCarriedApproval } from "../src/auto-approve.mjs"; import { mergeConfidence } from "../src/confidence.mjs"; const MARKER = ANY_REVIEW_MARKER; @@ -637,3 +641,75 @@ test("a reviewed head drops a limit note once the cap no longer applies", () => } assert.equal(bodyAtReviewedHead(body, 2), body); }); + +const FP_A = "a".repeat(64); +const FP_B = "b".repeat(64); +const HEAD_1 = "1".repeat(40); +const HEAD_2 = "2".repeat(40); +const stampedReview = (fingerprint = FP_A, extra = {}) => finishReviewBody("## 🔎 OpenObserve Code Review\n\n> [!TIP]\n> ### ✅ Approved", { + key: HEAD_1, passes: 1, maxPasses: 0, reviewed: true, confidence: { level: "high", reasons: ["no blockers"] }, diffFingerprint: fingerprint, ...extra, +}); + +test("a stamped review carries the fingerprint of the diff it read; an unstamped one vouches for none", () => { + assert.equal(reviewedDiffFingerprint(stampedReview()), FP_A); + assert.equal(reviewedDiffFingerprint(stampedReview(FP_A, { reviewed: false })), ""); + assert.equal(reviewedDiffFingerprint(stampedReview("")), ""); + assert.equal(reviewedDiffFingerprint(`plain text `), "", "without a sha tag no head was reviewed"); +}); + +test("an unchanged diff is skipped unless the run is forced or the diff is not pinned to the head", () => { + const body = stampedReview(); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_A, { pinned: true }), true); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_B, { pinned: true }), false); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_A, { pinned: false }), false); + assert.equal(shouldSkipAsUnchangedDiff(body, FP_A, { pinned: true, force: true }), false); + assert.equal(shouldSkipAsUnchangedDiff(body, "", { pinned: true }), false); +}); + +test("an unchanged-diff skip in approve mode keeps the review but not the auto-approval it can no longer back", () => { + const approved = autoApproveLine({ mode: "approve", approved: true, reasons: [] }); + const body = stampedReview(FP_A, { gateLine: approved }); + assert.ok(body.includes(approved)); + const next = withoutCarriedApproval(bodyAtUnchangedDiff(body, HEAD_2, 0)); + assert.equal(next.includes("Auto-approved"), false); + assert.match(next, /Needs human review/); + assert.equal(reviewedKey(next), HEAD_2); + assert.equal(reviewedDiffFingerprint(next), FP_A); + assert.equal(withoutCarriedApproval(stampedReview()), stampedReview(), "a body with no approval line is unchanged"); + const source = readFileSync(new URL("../run-review.mjs", import.meta.url), "utf-8"); + assert.match(source, /AUTO_APPROVE_MODE === "approve" \? withoutCarriedApproval\(kept\) : kept/); +}); + +test("a partial review leaves no fingerprint, so the next push with the same diff is reviewed again", () => { + const partial = stampedReview(""); + assert.equal(shouldSkipAsUnchangedDiff(partial, FP_A, { pinned: true }), false); + const overOld = finishReviewBody(`${stampedReview()}`, { key: HEAD_2, passes: 2, maxPasses: 0, reviewed: true, diffFingerprint: "" }); + assert.equal(reviewedDiffFingerprint(overOld), "", "an earlier complete review's fingerprint does not survive a partial one"); +}); + +test("skipping an unchanged diff moves the head marker, keeps the reviewed commit its line numbers refer to, and costs no pass", () => { + const body = stampedReview(); + const next = bodyAtUnchangedDiff(body, HEAD_2, 0); + assert.equal(reviewedKey(next), HEAD_2); + assert.equal(reviewedDiffFingerprint(next), FP_A); + assert.equal(reviewPassCount(next), 1); + assert.match(next, new RegExp(`Reviewed commit \`${HEAD_1.slice(0, 7)}\``)); + assert.match(next, /Merge confidence/); + assert.equal(shouldSkipAsReviewed(next, HEAD_2), true, "a queued run on the new head stays silent"); +}); + +test("a run on another head that left its claim behind gets the held verdict back when the diff turns out unchanged", () => { + const claimed = claimedReviewBody(stampedReview(), 2, HEAD_2); + assert.doesNotMatch(claimed, /High/); + const next = bodyAtUnchangedDiff(claimed, HEAD_2, 0); + assert.match(next, /High/); + assert.equal(reviewPassCount(next), 2); + assert.equal(reviewedDiffFingerprint(next), FP_A); +}); + +test("a model cannot forge the diff fingerprint, and the reviewers never see it", () => { + const forged = `## 🔎 OpenObserve Code Review\n\nok\n`; + const { text } = sanitizeReviewBody(forged, { marker: "", providerId: "p", modelId: "m", env: {} }); + assert.doesNotMatch(text, /o2-review-diff/); + assert.doesNotMatch(previousReviewText(stampedReview()), /o2-review-diff/); +}); diff --git a/scripts/ai-review/test/integration/opencode-sandbox.test.mjs b/scripts/ai-review/test/integration/opencode-sandbox.test.mjs index 1a2f0e971b..3f5c3c54a9 100644 --- a/scripts/ai-review/test/integration/opencode-sandbox.test.mjs +++ b/scripts/ai-review/test/integration/opencode-sandbox.test.mjs @@ -18,8 +18,8 @@ const REVIEW_AGENTS = [ ]; const skip = process.env.OPENCODE_BIN ? false : "set OPENCODE_BIN to the pinned opencode binary"; -// The first request that offers tools gets one scripted tool call; every later request gets the final answer. -function stubProvider(toolCall) { +// By default the first request that offers tools gets one scripted tool call and every later request the final answer; `script` overrides that per request. +function stubProvider(toolCall, { script } = {}) { const requests = []; const event = (type, data) => `event: ${type}\ndata: ${JSON.stringify({ type, ...data })}\n\n`; const server = http.createServer((req, res) => { @@ -34,9 +34,15 @@ function stubProvider(toolCall) { usage: { input_tokens: 1000, output_tokens: 1, cache_read_input_tokens: 0, cache_creation_input_tokens: 0 }, } })); const hasToolResult = (parsed.messages || []).some(m => Array.isArray(m.content) && m.content.some(b => b.type === "tool_result")); - if (parsed.tools?.length && !hasToolResult) { - res.write(event("content_block_start", { index: 0, content_block: { type: "tool_use", id: "toolu_1", name: toolCall.name, input: {} } })); - res.write(event("content_block_delta", { index: 0, delta: { type: "input_json_delta", partial_json: JSON.stringify(toolCall.args) } })); + const step = script ? script(parsed, requests.length) : { tool: parsed.tools?.length && !hasToolResult ? toolCall : null }; + if (step.hangMs) { + const timer = setTimeout(() => res.end(), step.hangMs); + res.on("close", () => clearTimeout(timer)); + return; + } + if (step.tool) { + res.write(event("content_block_start", { index: 0, content_block: { type: "tool_use", id: `toolu_${requests.length}`, name: step.tool.name, input: {} } })); + res.write(event("content_block_delta", { index: 0, delta: { type: "input_json_delta", partial_json: JSON.stringify(step.tool.args) } })); res.write(event("content_block_stop", { index: 0 })); res.write(event("message_delta", { delta: { stop_reason: "tool_use" }, usage: { output_tokens: 100 } })); } else { @@ -102,9 +108,10 @@ async function withServer(prDir, configPath, run, { readOnly = true } = {}) { } } -function stubConfig(file, stub, configDir) { +function stubConfig(file, stub, configDir, edit = () => {}) { const config = JSON.parse(stripJsonc(readFileSync(join(ENGINE_DIR, file), "utf-8"))); config.providers["anthropic-review"].settings = { baseURL: stub.url }; + edit(config); const configPath = join(configDir, "opencode.json"); writeFileSync(configPath, JSON.stringify(config)); return configPath; @@ -189,12 +196,17 @@ test("without sanitize, the env flags still keep project config, tools and agent assert.deepEqual(toolNames, ["glob", "grep", "read", "skill"]); }); -test("ci-full: every tool is offered and runs without a prompt, and the shell sees no GitHub token", { skip, timeout: 120_000 }, async t => { +const allowShell = (...commands) => config => { + config.agents["ai-review-correctness"].permissions.push(...commands.map(resource => ({ action: "shell", resource, effect: "allow" }))); +}; + +test("ci-full: file tools and the shell are offered without a prompt, and the shell sees no GitHub token", { skip, timeout: 120_000 }, async t => { const pr = plantedPr(); const stub = await stubProvider({ name: "shell", args: { command: "echo ran-$((6*7)); env", description: "probe" } }); const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); cleanup(t, stub, pr, configDir); - const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + // The allowlist refuses echo and env, so this probe of the shell's environment adds them for the test alone. + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir, allowShell("echo *", "env *")); sanitizePrTree(pr.root, ENGINE_DIR); await withServer(pr.root, configPath, async client => { @@ -206,8 +218,128 @@ test("ci-full: every tool is offered and runs without a prompt, and the shell se }, { readOnly: false }); const toolNames = (stub.requests[0].body.tools || []).map(tool => tool.name); - for (const name of ["read", "glob", "grep", "edit", "write", "shell", "webfetch"]) assert.ok(toolNames.includes(name), name); + for (const name of ["read", "glob", "grep", "edit", "write", "shell"]) assert.ok(toolNames.includes(name), name); + for (const name of ["webfetch", "websearch", "subagent", "question", "execute"]) assert.equal(toolNames.includes(name), false, `${name} is offered`); const content = toolResultOf(stub.requests.at(-1)); assert.match(content, /ran-42/); assert.doesNotMatch(content, /ghs_must_not_leak/); }); + +test("the real self-check passes the shipped ci-full config and refuses it with execute or the whole shell re-allowed", { skip, timeout: 120_000 }, async t => { + for (const [extra, problem] of [[null, null], [{ action: "execute", resource: "*", effect: "allow" }, /ai-review-qa: execute allowed/], [{ action: "shell", resource: "*", effect: "allow" }, /ai-review-qa: shell allows every command/]]) { + const pr = plantedPr(); + const stub = await stubProvider(null); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir, config => { if (extra) config.agents["ai-review-qa"].permissions.push(extra); }); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const check = selfCheck(client, REVIEW_AGENTS, { readOnly: false }); + if (problem) await assert.rejects(check, problem); + else assert.equal((await check).agents, REVIEW_AGENTS.length); + }, { readOnly: false }); + } +}); + +test("ci-full offers skill only because opencode appends a narrower rule, and refuses every skill", { skip, timeout: 120_000 }, async t => { + const pr = plantedPr(); + const stub = await stubProvider({ name: "skill", args: { id: "browser" } }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const result = await client.prompt({ ...PROMPT, title: "skill" }); + assert.equal(result.rejectedPermissions, 0); + }, { readOnly: false }); + assert.match(toolResultOf(stub.requests.at(-1)), /Permission denied: skill/); +}); + +test("ci-full shell runs an allowlisted command and refuses a build outright, without asking", { skip, timeout: 120_000 }, async t => { + // A RegExp is the output an allowed command must print; null means opencode must refuse it. + const ONE = /(? built.txt", null], ["cat /proc/self/environ", null], + ]; + for (const [command, output] of cases) { + const pr = plantedPr(); + const stub = await stubProvider({ name: "shell", args: { command, description: "probe" } }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const result = await client.prompt({ ...PROMPT, title: "shell" }); + assert.equal(result.rejectedPermissions, 0, "a denied command must not reach a prompt that ci-full would approve"); + }, { readOnly: false }); + const content = toolResultOf(stub.requests.at(-1)); + assert.match(content, output ?? /Permission denied: shell/, command); + assert.equal(existsSync(join(pr.root, "built.txt")), false, command); + } +}); + +// Why neither config sets `steps`: if opencode ever stops ending the capped turn on an assistant message, this fails and the cap can come back. +test("opencode 2.2.0's step cap ends the capped turn on an assistant prefill, which the model rejects with thinking on", { skip, timeout: 120_000 }, async t => { + const pr = plantedPr(); + // Asks for another tool call on every turn that allows one, so only the cap can end the loop. + const stub = await stubProvider(null, { script: parsed => ({ tool: parsed.tool_choice?.type === "none" ? null : { name: "glob", args: { pattern: "*.rs" } } }) }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir, config => { config.agents["ai-review-correctness"].steps = 3; }); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, client => client.prompt({ ...PROMPT, title: "steps" }), { readOnly: false }); + assert.equal(stub.requests.length, 3); + const capped = stub.requests.at(-1).body; + assert.deepEqual(capped.tool_choice, { type: "none" }); + assert.equal(capped.messages.at(-1).role, "assistant"); + assert.match(JSON.stringify(capped.messages.at(-1).content), /MAXIMUM STEPS REACHED/); + for (const request of stub.requests.slice(0, -1)) assert.equal(request.body.messages.at(-1).role, "user"); +}); + +test("each effort variant reaches the provider as its own effort level", { skip, timeout: 120_000 }, async t => { + for (const variant of ["low", "medium", "xhigh", "max"]) { + const pr = plantedPr(); + const stub = await stubProvider({ name: "glob", args: { pattern: "*.rs" } }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, client => client.prompt({ ...PROMPT, variant, title: variant }), { readOnly: false }); + for (const request of stub.requests) { + assert.deepEqual(request.body.output_config, { effort: variant }); + assert.equal(request.body.thinking?.type, "adaptive"); + for (const field of ["temperature", "top_p", "top_k", "tool_choice"]) assert.equal(request.body[field], undefined, field); + } + } +}); + +test("a session still running at its soft deadline is interrupted and answers the wrap-up prompt from the same context", { skip, timeout: 120_000 }, async t => { + const pr = plantedPr(); + const wrapText = "WRAP-UP-NOW-5d1e"; + // A tool step, then a model turn that never returns; only the wrap-up prompt gets an answer. + const stub = await stubProvider(null, { + script: (parsed, n) => { + if (parsed.messages?.some(m => JSON.stringify(m.content).includes(wrapText))) return { tool: null }; + return n === 1 ? { tool: { name: "glob", args: { pattern: "*.rs" } } } : { hangMs: 60_000 }; + }, + }); + const configDir = mkdtempSync(join(tmpdir(), "ai-review-config-")); + cleanup(t, stub, pr, configDir); + const configPath = stubConfig(OPENCODE_CONFIGS["ci-full"].file, stub, configDir); + sanitizePrTree(pr.root, ENGINE_DIR); + await withServer(pr.root, configPath, async client => { + const result = await client.prompt({ ...PROMPT, title: "wrap-up", timeoutMs: 20_000, wrapUp: { ms: 10_000, text: wrapText } }); + assert.equal(result.wrappedUp, true); + assert.equal(result.text, ""); + }, { readOnly: false }); + const last = stub.requests.at(-1); + assert.match(last.raw, new RegExp(wrapText)); + assert.ok(last.body.messages.some(m => Array.isArray(m.content) && m.content.some(b => b.type === "tool_result")), "the wrap-up lost the steps the agent had already run"); + for (const request of stub.requests) { + for (const field of ["temperature", "top_p", "top_k", "tool_choice"]) assert.equal(request.body[field], undefined, field); + assert.equal(request.body.messages.at(-1).role, "user", "the wrap-up must not end on an assistant prefill"); + } +}); diff --git a/scripts/ai-review/test/jsonc.mjs b/scripts/ai-review/test/jsonc.mjs index 670c1bbac1..9adbc3e55a 100644 --- a/scripts/ai-review/test/jsonc.mjs +++ b/scripts/ai-review/test/jsonc.mjs @@ -1,18 +1 @@ -// Comments and trailing commas out, so the opencode configs parse as JSON in tests. -export function stripJsonc(text) { - let out = ""; - for (let i = 0; i < text.length; i++) { - const c = text[i]; - if (c === '"') { - const start = i; - for (i++; i < text.length && text[i] !== '"'; i++) if (text[i] === "\\") i++; - out += text.slice(start, i + 1); - } else if (c === "/" && text[i + 1] === "/") { - while (i < text.length && text[i] !== "\n") i++; - out += "\n"; - } else { - out += c; - } - } - return out.replace(/,(\s*[}\]])/g, "$1"); -} +export { stripJsonc } from "../src/jsonc.mjs"; diff --git a/scripts/ai-review/test/opencode.test.mjs b/scripts/ai-review/test/opencode.test.mjs index 6e48eb3d32..cd1bdafeb1 100644 --- a/scripts/ai-review/test/opencode.test.mjs +++ b/scripts/ai-review/test/opencode.test.mjs @@ -11,15 +11,19 @@ import { OPENCODE_CONFIGS, OpencodeClient, buildServerEnv, + ciFullProblems, createSseParser, externalDirectoryProblem, httpRequest, isTransientError, + modelVariantIds, resolveOpencodeConfig, selfCheck, shouldRetry, toolAllowedWithoutAsking, toolEnabled, + wildcardMatch, + wrapUpStartMs, } from "../src/opencode.mjs"; import { stripJsonc } from "./jsonc.mjs"; @@ -28,15 +32,16 @@ const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "../../.."); const AGENT_RUN_MS = 1500; const HEADERS_TIMEOUT_MS = 1000; -function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execution.succeeded", error = null, errorBeforeAck = null, eventStream = true } = {}) { +function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execution.succeeded", error = null, errorBeforeAck = null, eventStream = true, wrapUpAfterMs = null, interruptEventDelayMs = 50 } = {}) { const calls = []; + let prompts = 0; const streams = new Set(); let idle = null; const emit = (type, data) => { for (const res of streams) res.write(`data: ${JSON.stringify({ id: "evt", type, data })}\n\n`); }; const finishRun = () => { - idle = { type: "idle", outcome: finalEvent === "session.execution.succeeded" ? "succeeded" : "failed", ...(error ? { error } : {}) }; + idle = { id: `idle_${prompts}`, type: "idle", outcome: finalEvent === "session.execution.succeeded" ? "succeeded" : "failed", ...(error ? { error } : {}) }; if (eventStream) emit(finalEvent, { sessionID: "ses_1", ...(error ? { error } : {}) }); }; const server = http.createServer((req, res) => { @@ -73,7 +78,13 @@ function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execut setTimeout(() => send(200, { data: { id: "msg_u1" } }), 200); return; } - send(200, { data: { id: "msg_u1" } }); + prompts += 1; + send(200, { data: { id: `msg_u${prompts}` } }); + // The second prompt is the wrap-up: it answers, where the first ran on until interrupted. + if (wrapUpAfterMs !== null && prompts > 1) { + setTimeout(finishRun, wrapUpAfterMs); + return; + } setTimeout(() => emit("permission.asked", { id: "per_1", sessionID: "ses_1", action: "shell", resources: ["*"] }), 50); if (idleAfterMs !== null) setTimeout(finishRun, idleAfterMs); return; @@ -103,7 +114,11 @@ function fakeOpencode({ idleAfterMs = AGENT_RUN_MS, finalEvent = "session.execut res.end(); return; } - if (req.method === "POST" && req.url === "/api/session/ses_1/interrupt") return send(200, { interrupted: true }); + if (req.method === "POST" && req.url === "/api/session/ses_1/interrupt") { + idle = { id: `idle_${prompts}`, type: "idle", outcome: "interrupted" }; + setTimeout(() => emit("session.execution.interrupted", { sessionID: "ses_1" }), interruptEventDelayMs); + return send(200, { interrupted: true }); + } if (req.method === "DELETE" && req.url === "/api/session/ses_1") { res.writeHead(204); res.end(); @@ -216,6 +231,61 @@ test("a timed-out agent is interrupted server-side and not reported as transient } }); +test("a reviewer still running near its timeout is interrupted and asked for its findings in the same session", async () => { + const fake = await fakeOpencode({ idleAfterMs: null, wrapUpAfterMs: 100 }); + try { + const client = new OpencodeClient({ baseUrl: fake.baseUrl, password: "pw", statusPollMs: 100 }); + const started = Date.now(); + const result = await client.prompt({ ...PROMPT, timeoutMs: 3000, wrapUp: { ms: 1000, text: "wrap up now" } }); + client.close(); + assert.ok(Date.now() - started < 3000, "waited for the hard timeout instead of the wrap-up answer"); + assert.equal(result.wrappedUp, true); + assert.equal(result.text, ""); + assert.equal(result.info.tokens.input, 2000, "tokens are the whole session's, both prompts included"); + const prompts = fake.calls.filter(c => c.url === "/api/session/ses_1/prompt"); + assert.deepEqual(prompts.map(c => c.body.text), ["", "wrap up now"]); + assert.equal(fake.calls.filter(c => c.url === "/api/session/ses_1/interrupt").length, 1); + } finally { + await fake.close(); + } +}); + +test("an interrupted event that arrives after the wrap-up prompt went out does not end the wrap-up", async () => { + // The poll confirms the interrupt first; the stream's interrupted event lands while the follow-up is running. + const fake = await fakeOpencode({ idleAfterMs: null, wrapUpAfterMs: 600, interruptEventDelayMs: 400 }); + try { + const client = new OpencodeClient({ baseUrl: fake.baseUrl, password: "pw", statusPollMs: 100 }); + const result = await client.prompt({ ...PROMPT, timeoutMs: 4000, wrapUp: { ms: 2000, text: "wrap up now" } }); + client.close(); + assert.equal(result.wrappedUp, true); + assert.equal(result.text, ""); + assert.equal(fake.calls.filter(c => c.url === "/api/session/ses_1/prompt").length, 2); + } finally { + await fake.close(); + } +}); + +test("a wrap-up that does not answer in its slot still ends as a hard timeout", async () => { + const fake = await fakeOpencode({ idleAfterMs: null }); + try { + const client = new OpencodeClient({ baseUrl: fake.baseUrl, password: "pw", statusPollMs: 100 }); + await assert.rejects(client.prompt({ ...PROMPT, timeoutMs: 1600, wrapUp: { ms: 800, text: "wrap up now" } }), err => err.name === "TimeoutError"); + client.close(); + assert.equal(fake.calls.filter(c => c.url === "/api/session/ses_1/prompt").length, 2); + } finally { + await fake.close(); + } +}); + +test("the wrap-up slot is taken only from a session at least twice its length", () => { + const wrapUp = { ms: 3 * 60_000, text: "wrap" }; + assert.equal(wrapUpStartMs(20 * 60_000, wrapUp), 17 * 60_000); + assert.equal(wrapUpStartMs(6 * 60_000, wrapUp), 3 * 60_000); + assert.equal(wrapUpStartMs(5 * 60_000, wrapUp), 5 * 60_000); + assert.equal(wrapUpStartMs(20 * 60_000, null), 20 * 60_000); + assert.equal(wrapUpStartMs(20 * 60_000, { ms: 60_000, text: "" }), 20 * 60_000); +}); + test("a provider rate limit surfaced as session.execution.failed is transient", async () => { const error = { type: "provider.rate-limit", message: "Too Many Requests", status: 429 }; const fake = await fakeOpencode({ idleAfterMs: 100, finalEvent: "session.execution.failed", error }); @@ -354,6 +424,14 @@ const READ_ONLY = [ rule("skill", "browser", "deny"), ]; const FULL = [...BASE, rule("*", "*", "allow"), rule("browser", "*", "deny")]; +const CI_FULL = [ + ...BASE, + rule("*", "*", "allow"), + ...["webfetch", "websearch", "subagent", "question", "execute", "skill"].map(id => rule(id, "*", "deny")), + rule("shell", "*", "deny"), + rule("shell", "git diff *", "allow"), + rule("browser", "*", "deny"), +]; const agent = (permissions, id = "ai-review-security") => ({ id, name: id, mode: "primary", permissions }); test("self-check passes only for read-only agents and built-in plugins", async () => { @@ -379,17 +457,21 @@ test("self-check passes only for read-only agents and built-in plugins", async ( await assert.rejects(selfCheck(fakeClient({ agents: [agent(FULL)] }), names), /shell allowed/, "the default config must refuse a full grant"); }); -test("ci-full self-check wants every agent registered with the full grant, and still refuses foreign plugins", async () => { +test("ci-full self-check wants file tools granted, web and sub-agent tools denied and the shell allowlisted", async () => { const names = ["ai-review-security", "ai-review-correctness"]; - const agents = [agent(FULL), agent(FULL, "ai-review-correctness")]; + const agents = [agent(CI_FULL), agent(CI_FULL, "ai-review-correctness")]; const result = await selfCheck(fakeClient({ agents }), names, { readOnly: false }); assert.equal(result.agents, 2); - await assert.rejects(selfCheck(fakeClient({ agents: [agent(FULL)] }), names, { readOnly: false }), /ai-review-correctness: not registered/); + await assert.rejects(selfCheck(fakeClient({ agents: [agent(CI_FULL)] }), names, { readOnly: false }), /ai-review-correctness: not registered/); await assert.rejects( - selfCheck(fakeClient({ agents: [agent(READ_ONLY), agent(FULL, "ai-review-correctness")] }), names, { readOnly: false }), + selfCheck(fakeClient({ agents: [agent(READ_ONLY), agent(CI_FULL, "ai-review-correctness")] }), names, { readOnly: false }), /ai-review-security: edit not fully allowed/, ); + assert.deepEqual(ciFullProblems(FULL), ["webfetch allowed", "websearch allowed", "subagent allowed", "question allowed", "execute allowed", "skill allowed", "shell allows every command"]); + assert.deepEqual(ciFullProblems([...CI_FULL, rule("execute", "*", "allow")]), ["execute allowed"]); + assert.deepEqual(ciFullProblems([...CI_FULL, rule("shell", "*", "deny")]), ["shell denied"]); + assert.deepEqual(ciFullProblems(CI_FULL), []); await assert.rejects( selfCheck(fakeClient({ agents, plugins: [...BUILTIN_PLUGINS, { id: "p", source: { type: "package", target: "evil" }, state: { status: "active" } }] }), names, { readOnly: false }), /non-built-in plugins/, @@ -427,10 +509,44 @@ test("both configs register the same agents, provider and model; only the defaul for (const toolId of BUILTIN_TOOL_IDS) assert.equal(toolEnabled(toolId, readOnly), ["read", "glob", "grep"].includes(toolId), `${name} ${toolId}`); assert.equal(externalDirectoryProblem(readOnly), "", name); const full = [...BASE, ...ci.agents[name].permissions]; - for (const toolId of BUILTIN_TOOL_IDS) assert.equal(toolAllowedWithoutAsking(toolId, full), true, `${name} ${toolId}`); + assert.deepEqual(ciFullProblems(full), [], name); + // opencode 2.2.0 ends the capped turn on an assistant message, a prefill the model rejects, so no agent sets a step cap. + for (const config of [base, ci]) for (const key of ["steps", "maxSteps"]) assert.equal(config.agents[name][key], undefined, `${name} ${key}`); } }); +// Evaluated as opencode 2.2.0 does: the last rule whose action and resource both match decides. +function shellDecision(rules, command) { + return [...BASE, ...rules].findLast(r => wildcardMatch("shell", r.action) && wildcardMatch(command, r.resource))?.effect; +} + +test("the ci-full shell runs the read-only allowlist and nothing that builds, installs or reaches the network", () => { + const ci = JSON.parse(stripJsonc(readFileSync(join(REPO_ROOT, OPENCODE_CONFIGS["ci-full"].file), "utf-8"))); + for (const [name, agentConfig] of Object.entries(ci.agents)) { + for (const command of ["git diff", "git diff origin/main -- src", "git log -5 --oneline", "git show HEAD:src/a.rs", "git blame -L 1,5 a.rs", "rg -n foo src", "cat a.rs", "ls", "ls -la web", "head -50 a.rs", "sed -n 1,40p a.rs", "wc -l a.rs", + "grep -rn foo src", "tail -n 20 a.rs", "find src -name '*.rs'", "find . -type f -newer a.rs", "find src -name '*-executor*'", "find . -name ok-delete.rs", + "git ls-files src", "git status", "git status --short", "git rev-parse HEAD", "git merge-base HEAD origin/main", "git cat-file -p HEAD:a.rs", "git cat-file -t HEAD", "git cat-file -s HEAD", + "git --no-pager diff --stat", "git --no-pager log -3", "git --no-pager show HEAD", "rg -n foo src/env_util.rs"]) { + assert.equal(shellDecision(agentConfig.permissions, command), "allow", `${name}: ${command}`); + } + for (const command of ["cargo build", "cargo test -p config", "npm ci", "npx vitest", "curl https://example.com", "git push", "git fetch origin", "git difftool", "sed -i s/a/b/ a.rs", "make", "python3 x.py", "bash -c ls", + "find . -exec rm {} ;", "find -execdir sh -c x ;", "find . -name x -ok rm {} ;", "find . -delete", "find . -fprint out.txt", "find . -fls out.txt", + "tail -f a.log", "tail -n 5 -F a.log", "tail --follow=name a.log", "git grep foo", "git grep -nOsh foo", "git grep --open-files=sh foo", "git --no-pager grep foo", + "git --no-pager push", "git --no-pager -c core.pager=sh log", "git cat-file --batch", "git cat-file blob HEAD", + "cat /proc/self/environ", "head -c 100 /proc/1/environ", "tail /dev/stdin", "grep -a KEY /proc/self/environ", "rg KEY /proc", "sed -n p /proc/self/environ", + "cat ../../proc/self/environ", "cat src/environ", "rg --file=/proc/self/environ x", "wc -c /p*/self/env*", "cat /etc/passwd", "cat a.rs > b.rs", "rg foo src>>out.txt", "cat a.rs 2>/dev/null"]) { + assert.equal(shellDecision(agentConfig.permissions, command), "deny", `${name}: ${command}`); + } + } +}); + +test("a trailing ' *' also matches the bare command, as in opencode 2.2.0", () => { + assert.equal(wildcardMatch("git diff", "git diff *"), true); + assert.equal(wildcardMatch("git diff --stat", "git diff *"), true); + assert.equal(wildcardMatch("git difftool", "git diff *"), false); + assert.equal(wildcardMatch("read", "*"), true); +}); + test("the review runs on the anthropic-review provider, the only one the policies allow, with the workflow's variant", () => { const base = JSON.parse(stripJsonc(readFileSync(join(REPO_ROOT, OPENCODE_CONFIGS.default.file), "utf-8"))); const allowed = base.policies.filter(p => p.action === "provider.use" && p.effect === "allow").map(p => p.resource); @@ -441,9 +557,14 @@ test("the review runs on the anthropic-review provider, the only one the policie assert.equal(provider.package, "@opencode/ai/providers/anthropic"); assert.deepEqual(provider.env, ["ANTHROPIC_REVIEW_API_KEY"]); const model = provider.models["claude-sonnet-5-5"]; - assert.deepEqual(model.variants, [{ id: "high", settings: { thinking: { type: "adaptive" }, effort: "high" } }]); + assert.deepEqual(model.variants, ["low", "medium", "high", "xhigh", "max"].map(id => ({ id, settings: { thinking: { type: "adaptive" }, effort: id } }))); + assert.deepEqual(modelVariantIds(base, "anthropic-review", "claude-sonnet-5-5"), ["low", "medium", "high", "xhigh", "max"]); + assert.deepEqual(modelVariantIds(base, "anthropic-review", "no-such-model"), []); const workflow = readFileSync(join(REPO_ROOT, ".github/workflows/ai-code-review.yml"), "utf-8"); - assert.match(workflow, /provider_id: anthropic-review\n\s+model_id: claude-sonnet-5-5\n\s+model_variant: high\n\s+api_key_env: ANTHROPIC_REVIEW_API_KEY\n/); + assert.match(workflow, /provider_id: anthropic-review\n\s+model_id: claude-sonnet-5-5\n\s+api_key_env: ANTHROPIC_REVIEW_API_KEY\n/); + assert.doesNotMatch(workflow, /model_variant/, "the effort variable is the only source of the variant"); + assert.match(workflow, /REVIEW_MODEL_VARIANT: \$\{\{ vars\.AI_REVIEW_EFFORT \|\| 'high' \}\}/); + assert.match(workflow, /REVIEW_COORDINATOR_VARIANT: \$\{\{ vars\.AI_REVIEW_COORDINATOR_EFFORT \|\| vars\.AI_REVIEW_EFFORT \|\| 'high' \}\}/); assert.match(workflow, /ANTHROPIC_REVIEW_API_KEY: \$\{\{ secrets\.USAGE_CREDIT_API_KEY \}\}/); }); diff --git a/scripts/ai-review/test/passes.test.mjs b/scripts/ai-review/test/passes.test.mjs index 72c022adfd..94680e6c96 100644 --- a/scripts/ai-review/test/passes.test.mjs +++ b/scripts/ai-review/test/passes.test.mjs @@ -1,5 +1,6 @@ import test from "node:test"; import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; import { mergeConfidence, unfinishedReviewers } from "../src/confidence.mjs"; import { reviewerFacts } from "../src/auto-approve.mjs"; @@ -10,6 +11,7 @@ import { parseColdPasses, passLabel, passPlan, + reviewComplete, runPooled, } from "../src/passes.mjs"; import { buildFallbackReview, partialFindingsBlock, selectFallbackFindings } from "../src/report.mjs"; @@ -137,3 +139,22 @@ test("the fallback never merges two different blockers that share most of their ]); assert.equal(selectFallbackFindings(same).length, 1); }); + +test("only a review whose every pass finished in time and whose coordinator merged may skip an unchanged diff next time", () => { + const coordinator = { fallback: false }; + const skippedPass = { agentKey: "frontend", agentName: "frontend Reviewer", pass: 1, findings: [], error: null, skipped: true }; + assert.equal(reviewComplete([done("security", 1), done("security", 2), skippedPass], coordinator), true); + assert.equal(reviewComplete([done("security", 1), failed("security", 2)], coordinator), false, "a lost cold pass is a partial review"); + assert.equal(reviewComplete([done("security", 1), failed("security", 2, { timedOut: true })], coordinator), false); + assert.equal(reviewComplete([done("security", 1), { ...done("qa", 1), parsed: false }], coordinator), false, "unparseable output is unfinished"); + assert.equal(reviewComplete([done("security", 1), { ...done("qa", 1), wrappedUp: true }], coordinator), false, "a reviewer cut short at its soft deadline"); + assert.equal(reviewComplete([done("security", 1)], { fallback: true }), false, "the coordinator fell back"); + assert.equal(reviewComplete([done("security", 1)], null), false, "the coordinator never ran"); +}); + +test("run-review writes the diff fingerprint only for a complete review", () => { + const source = readFileSync(new URL("../run-review.mjs", import.meta.url), "utf-8"); + assert.match(source, /const complete = reviewComplete\(passResults, coordinatorResult\);/); + assert.match(source, /diffFingerprint: complete && fullyRead && diffPinned && hunksLocatable\(diff, readBaseFile\) \? fingerprint : ""/); + assert.equal((source.match(/diffFingerprint:/g) || []).length, 1, "every finishReviewBody call goes through the completeness check"); +}); diff --git a/scripts/ai-review/test/pr-diff.test.mjs b/scripts/ai-review/test/pr-diff.test.mjs index 2d7551e9a1..dc75c557f8 100644 --- a/scripts/ai-review/test/pr-diff.test.mjs +++ b/scripts/ai-review/test/pr-diff.test.mjs @@ -6,7 +6,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { parseRawDiff, sizeReasons } from "../src/auto-approve.mjs"; -import { fetchDiff, fetchPinnedDiff, localDiff, pinDiff, prefixDiffPaths } from "../src/pr-diff.mjs"; +import { diffFingerprint, fetchDiff, fetchPinnedDiff, hunksLocatable, localDiff, pinDiff, prefixDiffPaths } from "../src/pr-diff.mjs"; const DIFF = [ "diff --git a/src/lib.rs b/src/lib.rs", @@ -199,3 +199,114 @@ test("a renamed symlink or submodule whose header carries no mode cannot pass as assert.match(edited, /^index \S+ 100644$/m); assert.deepEqual(parseRawDiff(edited).problems, []); }); + +test("a rebase that moves the PR's lines keeps its diff fingerprint, a changed line does not", () => { + const dir = mkdtempSync(join(tmpdir(), "ai-review-fingerprint-")); + const git = (...args) => execFileSync("git", ["-C", dir, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", ...args], { encoding: "utf-8" }); + try { + git("init", "-q", "-b", "main"); + const lines = Array.from({ length: 40 }, (_, i) => `line ${i}`); + writeFileSync(join(dir, "a.rs"), `${lines.join("\n")}\n`); + git("add", "."); + git("commit", "-qm", "base"); + git("checkout", "-qb", "pr"); + writeFileSync(join(dir, "a.rs"), `${lines.map((l, i) => (i === 30 ? "line 30 changed" : l)).join("\n")}\n`); + git("commit", "-qam", "pr change"); + const before = localDiff(dir, "main", "pr"); + git("checkout", "-q", "main"); + writeFileSync(join(dir, "a.rs"), `head 1\nhead 2\nhead 3\n${lines.join("\n")}\n`); + git("commit", "-qam", "main moves on"); + git("checkout", "-q", "pr"); + git("rebase", "-q", "main"); + const after = localDiff(dir, "main", "pr"); + assert.notEqual(before, after, "the rebase must change hunk numbers and blob ids, or this test proves nothing"); + assert.equal(diffFingerprint(after), diffFingerprint(before)); + writeFileSync(join(dir, "a.rs"), `head 1\nhead 2\nhead 3\n${lines.map((l, i) => (i === 30 ? "line 30 changed again" : l)).join("\n")}\n`); + git("commit", "-qam", "another change"); + assert.notEqual(diffFingerprint(localDiff(dir, "main", "pr")), diffFingerprint(before)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("the fingerprint ignores only blob ids and hunk numbers", () => { + const base = "diff --git a/a b/a\nindex 1111111..2222222 100644\n--- a/a\n+++ b/a\n@@ -1,2 +1,2 @@ fn x\n ctx\n-old\n+new\n"; + assert.equal(diffFingerprint(base.replace("1111111..2222222", "3333333..4444444").replace("-1,2 +1,2 @@ fn x", "-9,2 +12,2 @@ fn x")), diffFingerprint(base)); + assert.notEqual(diffFingerprint(base.replace("@@ fn x", "@@ fn y")), diffFingerprint(base), "the function a hunk sits in is part of what was reviewed"); + assert.notEqual(diffFingerprint(base.replace("+new", "+newer")), diffFingerprint(base)); + assert.notEqual(diffFingerprint(base.replace("diff --git a/a b/a", "diff --git a/b b/b")), diffFingerprint(base)); + assert.match(diffFingerprint(base), /^[0-9a-f]{64}$/); +}); + +test("the same lines changed inside a different function get a different fingerprint", () => { + const dir = mkdtempSync(join(tmpdir(), "ai-review-fingerprint-")); + const git = (...args) => execFileSync("git", ["-C", dir, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", ...args], { encoding: "utf-8" }); + const fn = (name, allowed = "user.active") => `function ${name}(user) {\n const id = user.id;\n const name = user.name;\n const role = user.role;\n const group = user.group;\n const allowed = ${allowed};\n const details = { id, name, role, group };\n const present = Boolean(details);\n const result = Boolean(allowed);\n return present && result;\n}\n`; + try { + git("init", "-q", "-b", "main"); + writeFileSync(join(dir, "rules.js"), `${fn("preview")}\n${fn("authorize")}`); + git("add", "."); + git("commit", "-qm", "base"); + git("checkout", "-qb", "safe"); + writeFileSync(join(dir, "rules.js"), `${fn("preview", "true")}\n${fn("authorize")}`); + git("commit", "-qam", "safe"); + git("checkout", "-qb", "unsafe", "main"); + writeFileSync(join(dir, "rules.js"), `${fn("preview")}\n${fn("authorize", "true")}`); + git("commit", "-qam", "unsafe"); + const safe = localDiff(dir, "main", "safe"); + const unsafe = localDiff(dir, "main", "unsafe"); + assert.match(safe, /^@@ .* @@ function preview\(user\) \{$/m); + assert.match(unsafe, /^@@ .* @@ function authorize\(user\) \{$/m); + assert.notEqual(diffFingerprint(unsafe), diffFingerprint(safe)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("an edit to one of two identical blocks in a function cannot be located, so it is never taken for the reviewed one", () => { + const dir = mkdtempSync(join(tmpdir(), "ai-review-fingerprint-")); + const git = (...args) => execFileSync("git", ["-C", dir, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false", ...args], { encoding: "utf-8" }); + const block = (flag, allowed) => [` if (${flag}) {`, " const a = 1;", " const b = 2;", " const c = 3;", ` ok = ${allowed};`, " const d = 4;", " const e = 5;", " const f = 6;", " }"]; + const source = (first, second) => ["function check(user, internal) {", " let ok = false;", ...block("internal", first), ...block("!internal", second), " return ok;", "}", ""].join("\n"); + const readBase = ref => path => { try { return execFileSync("git", ["-C", dir, "cat-file", "blob", `${ref}:${path}`], { encoding: "utf-8" }); } catch { return null; } }; + try { + git("init", "-q", "-b", "main"); + writeFileSync(join(dir, "check.js"), source("user.admin", "user.admin")); + git("add", "."); + git("commit", "-qm", "base"); + git("checkout", "-qb", "internal"); + writeFileSync(join(dir, "check.js"), source("true", "user.admin")); + git("commit", "-qam", "internal"); + git("checkout", "-qb", "external", "main"); + writeFileSync(join(dir, "check.js"), source("user.admin", "true")); + git("commit", "-qam", "external"); + const internal = localDiff(dir, "main", "internal"); + const external = localDiff(dir, "main", "external"); + assert.notEqual(internal, external); + assert.equal(hunksLocatable(external, readBase("main")), false, "the edited block's old text sits twice in the base"); + assert.equal(hunksLocatable(internal, readBase("main")), false); + writeFileSync(join(dir, "other.js"), "a\nb\nc\nd\ne\nf\ng\n"); + git("add", "other.js"); + git("commit", "-qm", "unique"); + writeFileSync(join(dir, "other.js"), "a\nb\nc\nD\ne\nf\ng\n"); + git("commit", "-qam", "edit unique"); + const unique = localDiff(dir, "HEAD~1", "HEAD"); + assert.equal(hunksLocatable(unique, readBase("HEAD~1")), true); + assert.equal(hunksLocatable(unique, () => null), false, "an unreadable base file cannot locate the edit"); + assert.equal(hunksLocatable("diff --git a/n b/n\nnew file mode 100644\n--- /dev/null\n+++ b/n\n@@ -0,0 +1 @@\n+x\n", () => null), true, "a new file has nothing to locate"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a changed binary file or trailing space on the last line changes the fingerprint", () => { + const bin = id => `diff --git a/m.wasm b/m.wasm\nindex 1111111..${id} 100644\nBinary files a/m.wasm and b/m.wasm differ\n`; + assert.notEqual(diffFingerprint(bin("2222222")), diffFingerprint(bin("3333333"))); + const csv = tail => `diff --git a/a.csv b/a.csv\nindex 1111111..2222222 100644\n--- a/a.csv\n+++ b/a.csv\n@@ -1 +1 @@\n-x\n+secret${tail}\n`; + assert.notEqual(diffFingerprint(csv(" ")), diffFingerprint(csv(" "))); +}); + +test("a file mode change keeps a different fingerprint", () => { + const d = mode => `diff --git a/l b/l\nindex 1111111..2222222 ${mode}\n--- a/l\n+++ b/l\n@@ -1 +1 @@\n-old-target\n\\ No newline at end of file\n+new-target\n\\ No newline at end of file\n`; + assert.notEqual(diffFingerprint(d("100644")), diffFingerprint(d("120000"))); +}); diff --git a/scripts/ai-review/test/report.test.mjs b/scripts/ai-review/test/report.test.mjs index 6d796b5bc9..0210efb8fb 100644 --- a/scripts/ai-review/test/report.test.mjs +++ b/scripts/ai-review/test/report.test.mjs @@ -37,7 +37,8 @@ import { } from "../src/report.mjs"; import { extractInlineBlock } from "../src/inline.mjs"; import { LOADER_DIRS, LOADER_FILES } from "../src/sanitize.mjs"; -import { reviewerDiff } from "../src/pr-diff.mjs"; +import { diffFingerprint, hunksLocatable, reviewerDiff } from "../src/pr-diff.mjs"; +import { narrowTier } from "../src/selection.mjs"; import * as approvalModule from "../src/auto-approve.mjs"; import * as commentsModule from "../src/comments.mjs"; import * as confidenceModule from "../src/confidence.mjs"; @@ -114,6 +115,13 @@ test("severityRank orders p0 first and treats legacy words like their mapped lev const xmlFinding = (severity, summary, extra = "") => `security${severity}src/a.rs3${summary}ds${extra}`; const reviewer = (findings, agentName = "Security Reviewer") => ({ agentName, findings: parseFindings(findings.join("\n")) }); +test("the fallback calls P1-only findings major concerns, still blocking", () => { + const review = buildFallbackReview([reviewer([xmlFinding("p1", "high"), xmlFinding("p2", "mid")])], "full", [], ""); + assert.match(review, /> \[!CAUTION\]\n> ### ⚠️ Major concerns found\n/); + assert.ok(review.includes("#### 🟠 P1 · Major concerns")); + assert.equal(review.includes("Blockers"), false); +}); + test("findings are normalized when parsed, and one with no severity is dropped", () => { const findings = parseFindings([xmlFinding("Critical", "a"), xmlFinding("P1", "b"), xmlFinding("", "c")].join("")); assert.deepEqual(findings.map(f => [f.summary, f.severity]), [["a", "p0"], ["b", "p1"]]); @@ -128,8 +136,8 @@ test("the fallback groups into P0 to P3 in order, with the caps applied", () => ])]; const review = buildFallbackReview(results, "full", [], ""); const at = text => review.indexOf(text); - assert.ok(at("#### 🔴 P0 · Blockers") < at("#### 🟠 P1 · Blockers")); - assert.ok(at("#### 🟠 P1 · Blockers") < at("#### 🟡 P2 · Should fix")); + assert.ok(at("#### 🔴 P0 · Blockers") < at("#### 🟠 P1 · Major concerns")); + assert.ok(at("#### 🟠 P1 · Major concerns") < at("#### 🟡 P2 · Should fix")); assert.ok(at("#### 🟡 P2 · Should fix") < at("#### 🔵 P3 · Worth a look")); assert.match(review, /> \[!CAUTION\]\n> ### ⛔ Blockers found\n> Automated review .*un-judged\./); assert.match(review, /\*\*🔴 P0: 1 · 🟠 P1: 1 · 🟡 P2: 3 · 🔵 P3: 2\*\*/); @@ -162,8 +170,8 @@ test("merge confidence applies every rule, the lowest level wins and every cause ["clean", {}, "high", ["no blockers; all reviewers finished; tests changed"]], ["no production code", { prodChanged: false, testsChanged: false }, "high", ["no blockers; all reviewers finished"]], ["p3 alone", { counts: { p3: 2 } }, "high", ["no blockers; all reviewers finished; tests changed"]], - ["one blocker", { counts: { p1: 1 } }, "low", ["1 blocker (P0/P1)"]], - ["two blockers", { counts: { p0: 1, p1: 1 } }, "low", ["2 blockers (P0/P1)"]], + ["one major concern", { counts: { p1: 1 } }, "low", ["1 major concern (P1)"]], + ["a blocker and a major concern", { counts: { p0: 1, p1: 1 } }, "low", ["1 blocker (P0)", "1 major concern (P1)"]], ["uncounted", { findingsKnown: false }, "low", ["findings could not be counted"]], ["security unfinished", { failedReviewers: ["Security Reviewer"] }, "low", ["Security reviewer did not finish"]], ["correctness unfinished", { failedReviewers: ["Correctness Reviewer"] }, "low", ["Correctness reviewer did not finish"]], @@ -175,7 +183,7 @@ test("merge confidence applies every rule, the lowest level wins and every cause ["all failed", { failedReviewers: ["Security Reviewer", "Release Reviewer"], ranCount: 2 }, "low", ["no reviewer finished"]], ["none ran", { ranCount: 0 }, "low", ["no reviewer ran"]], ["lowest wins, worst first", { counts: { p1: 1, p2: 1 }, testsChanged: false, failedReviewers: ["Release Reviewer"] }, "low", - ["1 blocker (P0/P1)", "Release reviewer did not finish", "1 P2 finding", "no tests changed"]], + ["1 major concern (P1)", "Release reviewer did not finish", "1 P2 finding", "no tests changed"]], ]; for (const [name, change, level, reasons] of table) { assert.deepEqual(mergeConfidence({ ...clean, ...change }), { level, reasons }, name); @@ -469,7 +477,7 @@ test("an approved callout over a visible blocker section or count never approves assert.equal(isApprovedReview(`${clean}\n\n**Merge confidence: 🟢 High** — no blockers; all reviewers finished`), true); const blocked = [ `${clean}\n\n#### 🔴 P0 · Blockers\n- \`a.rs:1\` **[Security]** Leaks a token.`, - `${clean}\n\n
\n\n#### 🟠 P1 · Blockers\n- \`a.rs:1\` **[Correctness]** Crashes.\n
`, + `${clean}\n\n
\n\n#### 🟠 P1 · Major concerns\n- \`a.rs:1\` **[Correctness]** Crashes.\n
`, `${head}**🔴 P0: 1 · 🟠 P1: 0 · 🟡 P2: 0 · 🔵 P3: 0**`, `${head}**🔴 P0: 0 · 🟠 P1: 2 · 🟡 P2: 0 · 🔵 P3: 0**`, `${head}**🔴 1 blocker · 🟡 0 warnings**`, @@ -779,7 +787,8 @@ const mainSource = () => { const source = readFileSync(new URL("../run-review.mjs", import.meta.url), "utf-8"); const fetcher = source.slice(source.indexOf("function fetchPrDiff("), source.indexOf("function pairedPrContext(")); const notes = source.slice(source.indexOf("function noteOnComment("), source.indexOf("function noteSkippedPush(")); - return `${fetcher}\n${notes}\n${source.slice(source.indexOf("async function main() {"), source.indexOf("// ─── Entry point"))}\nreturn main;`; + const unchanged = source.slice(source.indexOf("function unchangedDiffCheck("), source.indexOf("function prepareReviewTree(")); + return `${fetcher}\n${notes}\n${unchanged}\n${source.slice(source.indexOf("async function main() {"), source.indexOf("// ─── Entry point"))}\nreturn main;`; }; const cleanCoordinatorOutput = `${commentsModule.REVIEW_HEADING}\n\n> [!TIP]\n> ### ✅ Approved\n> Looks right.\n\n`; const finishedReviewer = key => ({ agentKey: key, agentName: `${key} Reviewer`, findings: [], parsed: true, fullDiff: true, error: null }); @@ -787,19 +796,19 @@ const silentReviewer = key => ({ ...finishedReviewer(key), parsed: false, rawTex const smallRawDiff = "diff --git a/docs/a.md b/docs/a.md\n--- a/docs/a.md\n+++ b/docs/a.md\n@@ -1 +1 @@\n-old\n+new\n"; // Every default makes the run an approvable one, so a test changes the single fact it is about. -async function runMain({ results = [finishedReviewer("security"), finishedReviewer("correctness")], byPass = null, pinned = {}, mode = "approve", existing = null, reviewedBody = "plain words", prNow = {}, paired = {}, contextFails = false, noiseOnly = false, diffFails = false, coordinatorOutput = cleanCoordinatorOutput, rawDiff = smallRawDiff, keptDiff = null } = {}) { +async function runMain({ results = [finishedReviewer("security"), finishedReviewer("correctness")], byPass = null, pinned = {}, mode = "approve", existing = null, reviewedBody = "plain words", prNow = {}, paired = {}, contextFails = false, noiseOnly = false, diffFails = false, manual = false, head = MAIN_HEAD, coordinatorOutput = cleanCoordinatorOutput, rawDiff = smallRawDiff, keptDiff = null, variantError = "", ignorePassLimit = false, forceFull = false, baseFile = "old\n" } = {}) { const run = { posted: "", approvals: [], logs: [], coordinatorCalls: 0, coordinatorArgs: null, reviewerCalls: [], artifact: null, updates: 0, creates: 0, claims: 0, sanitized: 0 }; - const pr = { ...eligibleInput().pr, title: "fix a bug", body: "plain words", head: { sha: MAIN_HEAD }, changed_files: 1, additions: 1, deletions: 1, ...prNow }; + const pr = { ...eligibleInput().pr, title: "fix a bug", body: "plain words", head: { sha: head }, changed_files: 1, additions: 1, deletions: 1, ...prNow }; const noop = () => {}; const scope = { ...commentsModule, ...confidenceModule, ...approvalModule, ...reportModule, ...passesModule, REVIEW_COLD_PASSES: 2, REVIEWER_CONCURRENCY: 16, - process: { env: { PR_NUMBER: "1", GITHUB_REPOSITORY: "test/repo", HEAD_SHA: MAIN_HEAD, MERGE_BASE_SHA: "c".repeat(40) } }, + process: { env: { PR_NUMBER: "1", GITHUB_REPOSITORY: "test/repo", HEAD_SHA: head, MERGE_BASE_SHA: "c".repeat(40), ...(forceFull ? { FORCE_FULL: "true" } : {}) } }, console: { log: (...args) => run.logs.push(args.join(" ")), warn: noop, error: noop }, TRACE: { startSpan: () => ({}), endSpan: noop, setSpanAttributes: noop }, activeRootSpan: null, isoNow: () => "now", prepareReviewTree: noop, findExistingReviewComment: () => ({ comment: existing, ok: true }), passesDoneIn: comment => (comment ? commentsModule.reviewPassCount(comment.body) : 0), - REVIEW_IGNORE_PASS_LIMIT: false, REVIEW_MAX_PASSES: 0, REVIEW_BOT_LOGIN: "review[bot]", REVIEW_PAIR_CLEAR: true, AUTO_APPROVE_MODE: mode, + REVIEW_IGNORE_PASS_LIMIT: ignorePassLimit, REVIEW_MANUAL_RUN: manual, MAX_DIFF_TOKENS: 150_000, diffFingerprint, hunksLocatable, readBaseFile: () => baseFile, variantProblem: () => variantError, MODEL_VARIANT: "high", COORDINATOR_VARIANT: "high", REVIEW_MAX_PASSES: 0, REVIEW_BOT_LOGIN: "review[bot]", REVIEW_PAIR_CLEAR: true, AUTO_APPROVE_MODE: mode, REVIEW_MARKER: "", PAIRED: { repo: "", prNumber: "", dir: "", mergeBase: "", head: "", ...paired }, PR_DIR: "/pr", resolve: (...parts) => parts.join("/"), githubReviewApi: () => ({ readPr: () => pr, listReviews: () => [], dismissReview: noop, approve: payload => run.approvals.push(payload) }), noteSkippedPush: noop, apiKey: () => "configured", API_KEY_VAR_NAME: "TEST_KEY", MODEL_SLUG: "test/model", @@ -922,6 +931,109 @@ test("a push the run stops on before claiming a pass loses the old verdict and k } }); +test("a new head whose diff matches the reviewed one is skipped without a pass, a model call or a claim", async () => { + const first = await runMain({ mode: "off" }); + assert.equal(commentsModule.reviewedDiffFingerprint(first.posted), diffFingerprint(smallRawDiff)); + const rebased = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD }); + assert.equal(rebased.error, undefined); + assert.ok(rebased.logs.some(line => line.includes("unchanged_diff"))); + assert.deepEqual(rebased.reviewerCalls, []); + assert.equal(rebased.coordinatorCalls, 0); + assert.equal(rebased.claims, 0); + assert.equal(rebased.updates, 1); + assert.equal(commentsModule.reviewedKey(rebased.posted), PAIRED_MAIN_HEAD); + assert.equal(commentsModule.reviewPassCount(rebased.posted), 1); + assert.match(rebased.posted, /Merge confidence: 🟢 High/); +}); + +test("a hand-started run, a changed diff or an unpinned diff is reviewed even after a matching review", async () => { + const first = await runMain({ mode: "off" }); + const existing = { id: 1, body: first.posted }; + const changed = `${smallRawDiff}+more\n`; + for (const run of [ + await runMain({ mode: "off", existing, head: PAIRED_MAIN_HEAD, manual: true }), + await runMain({ mode: "off", existing, head: PAIRED_MAIN_HEAD, rawDiff: changed }), + await runMain({ mode: "off", existing, head: PAIRED_MAIN_HEAD, pinned: { "test/repo": false } }), + ]) { + assert.equal(run.logs.some(line => line.includes("unchanged_diff")), false); + assert.equal(run.claims, 1); + assert.ok(run.reviewerCalls.length > 0); + assert.equal(commentsModule.reviewPassCount(run.posted), 2); + } +}); + +test("a hand-started, force-full or pass-limit-ignoring run reviews the head the last review already covered", async () => { + const first = await runMain({ mode: "off" }); + const existing = { id: 1, body: first.posted }; + for (const bypass of [{ manual: true }, { forceFull: true }, { ignorePassLimit: true }]) { + const run = await runMain({ mode: "off", existing, ...bypass }); + assert.equal(run.error, undefined, JSON.stringify(bypass)); + assert.equal(run.logs.some(line => /already_reviewed|unchanged_diff/.test(line)), false, JSON.stringify(bypass)); + assert.ok(run.reviewerCalls.length > 0, JSON.stringify(bypass)); + assert.equal(run.claims, 1, JSON.stringify(bypass)); + } + const automatic = await runMain({ mode: "off", existing }); + assert.ok(automatic.logs.some(line => line.includes("already_reviewed"))); + assert.deepEqual(automatic.reviewerCalls, []); +}); + +test("reviewers cut short at their soft deadline are not finished, so the verdict is not High and the gate does not approve", async () => { + const wrapped = key => ({ ...finishedReviewer(key), wrappedUp: true }); + for (const results of [[wrapped("security"), wrapped("correctness")], [finishedReviewer("security"), wrapped("correctness")]]) { + const run = await runMain({ results }); + assert.equal(run.error, undefined); + assert.equal(run.approvals.length, 0); + assert.doesNotMatch(run.posted, /Merge confidence: 🟢 High/); + assert.doesNotMatch(run.posted, /all reviewers finished/); + assert.doesNotMatch(run.posted, /Auto-approved/); + assert.equal(commentsModule.reviewedDiffFingerprint(run.posted), ""); + } + const byPass = (key, pass) => ({ ...finishedReviewer(key), wrappedUp: pass === 2 }); + const oneCleanPass = await runMain({ byPass }); + assert.equal(oneCleanPass.approvals.length, 1, "an area with one uninterrupted pass is still reviewed"); +}); + +test("an invalid effort setting retracts the old verdict and approval line without claiming a pass", async () => { + const first = await runMain(); + assert.match(first.posted, /Merge confidence: 🟢 High/); + assert.match(first.posted, /Auto-approved/); + const variantError = 'variant "typo" is not declared for test/model in opencode.ci.jsonc (declared: high)'; + for (const stop of [{ head: PAIRED_MAIN_HEAD, rawDiff: `${smallRawDiff}+more\n` }, { manual: true }]) { + const run = await runMain({ existing: { id: 1, body: first.posted }, variantError, ...stop }); + assert.match(run.error, /variant "typo" is not declared/); + assert.equal(run.claims, 0); + assert.deepEqual(run.reviewerCalls, []); + assert.equal(run.updates, 1); + assert.doesNotMatch(run.posted, /High/); + assert.doesNotMatch(run.posted, /Auto-approved/); + assert.match(run.posted, /the review did not run/); + assert.doesNotMatch(run.posted, /typo|test\/model/, "the configuration detail stays in the log"); + assert.equal(commentsModule.reviewPassCount(run.posted), 1); + assert.equal(commentsModule.reviewedKey(run.posted), MAIN_HEAD); + } +}); + +test("an unchanged diff whose hunk text appears twice in the base is reviewed, as the edit cannot be located", async () => { + const first = await runMain({ mode: "off" }); + const twice = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD, baseFile: "old\nx\nold\n" }); + assert.equal(twice.logs.some(line => line.includes("unchanged_diff")), false); + assert.equal(twice.claims, 1); + assert.equal(commentsModule.reviewedDiffFingerprint(twice.posted), "", "a review whose edit cannot be located records no fingerprint"); + const unreadable = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD, baseFile: null }); + assert.equal(unreadable.claims, 1); + assert.ok((await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD })).logs.some(line => line.includes("unchanged_diff"))); +}); + +test("a push that only adds a production asset the noise filter hides is reviewed, not skipped with the old High", async () => { + const first = await runMain({ mode: "off" }); + assert.match(first.posted, /Merge confidence: 🟢 High/); + const svg = "diff --git a/web/src/assets/chart.svg b/web/src/assets/chart.svg\nnew file mode 100644\n--- /dev/null\n+++ b/web/src/assets/chart.svg\n@@ -0,0 +1 @@\n+\n"; + const next = await runMain({ mode: "off", existing: { id: 1, body: first.posted }, head: PAIRED_MAIN_HEAD, rawDiff: `${smallRawDiff}${svg}`, keptDiff: smallRawDiff }); + assert.equal(next.logs.some(line => line.includes("unchanged_diff")), false); + assert.equal(next.claims, 1); + assert.doesNotMatch(next.posted, /Merge confidence: 🟢 High/); +}); + test("a dangling second block cannot read as a clean count, so the gate refuses", () => { const output = `${cleanCoordinatorOutput}\n + + + + + diff --git a/web/src/components/QueryEditor.spec.ts b/web/src/components/QueryEditor.spec.ts index 1bfbc80c9b..06d75e1ab8 100644 --- a/web/src/components/QueryEditor.spec.ts +++ b/web/src/components/QueryEditor.spec.ts @@ -68,6 +68,7 @@ const codeQueryEditorStub = { "editorId", "valuesTimeRange", "textPreview", + "transformGeneratedQuery", ], emits: [ "update:query", @@ -581,6 +582,29 @@ describe("QueryEditor", () => { }); }); + describe("page hooks for the code editor", () => { + it("names the editor from the prefix and language by default", () => { + wrapper = mountQueryEditor({ dataTestPrefix: "logs-search-bar" }); + expect(wrapper.findComponent(codeQueryEditorStub).props("editorId")).toBe( + "logs-search-bar-editor-sql", + ); + expect( + wrapper.findComponent(codeQueryEditorStub).props("transformGeneratedQuery"), + ).toBeUndefined(); + }); + + it("forwards a page's editor id and generated-query transform", () => { + const transform = (sql: string) => sql; + wrapper = mountQueryEditor({ + editorId: "traces-query-editor", + transformGeneratedQuery: transform, + }); + const editor = wrapper.findComponent(codeQueryEditorStub); + expect(editor.props("editorId")).toBe("traces-query-editor"); + expect(editor.props("transformGeneratedQuery")).toBe(transform); + }); + }); + describe("values time range", () => { it("forwards the range to the code editor", () => { const range = { startTime: 1, endTime: 2 }; diff --git a/web/src/components/QueryEditor.vue b/web/src/components/QueryEditor.vue index 1e0898fce4..8c275c3934 100644 --- a/web/src/components/QueryEditor.vue +++ b/web/src/components/QueryEditor.vue @@ -85,7 +85,7 @@ string | null) | null; // Page-specific write of an AI result (null result = keep the editor) // NL Mode (optional external control) nlpMode?: boolean; // External NLP mode control (undefined = internal control) @@ -208,6 +210,7 @@ interface Props { // Testing dataTestPrefix?: string; + editorId?: string; // Monaco host id; defaults to `-editor-` } const props = withDefaults(defineProps(), { diff --git a/web/src/components/alerts/IncidentTimeline.spec.ts b/web/src/components/alerts/IncidentTimeline.spec.ts index ac6ff1c6e7..6cc00dc293 100644 --- a/web/src/components/alerts/IncidentTimeline.spec.ts +++ b/web/src/components/alerts/IncidentTimeline.spec.ts @@ -353,6 +353,15 @@ describe("IncidentTimeline - getInlineEventText", () => { expect(text).toContain("5 times"); }); + /// O1: a page Resolve that ended the incident's ack says so, not "Reopened". + it("says a page Resolve ended the acknowledgement", async () => { + const w = await mountComp(); + await flushPromises(); + const event = { type: "AckEndedByPageResolve", data: { user_id: "ana@o2.ai" } }; + expect((w.vm as any).getEventBadgeText(event)).toBe("Acknowledgement ended"); + expect((w.vm as any).getInlineEventText(event)).toBe("by resolving one of its pages"); + }); + it("returns text for ai_analysis_begin", async () => { const w = await mountComp(); await flushPromises(); diff --git a/web/src/components/alerts/IncidentTimeline.vue b/web/src/components/alerts/IncidentTimeline.vue index 3367156de7..d86bb1c53c 100644 --- a/web/src/components/alerts/IncidentTimeline.vue +++ b/web/src/components/alerts/IncidentTimeline.vue @@ -160,6 +160,8 @@ const getEventIcon = (event: any): string => { return "check"; case "Reopened": return "replay"; + case "AckEndedByPageResolve": + return "check"; case "DimensionsUpgraded": return "arrow-upward"; case "TitleChanged": @@ -201,6 +203,8 @@ const getEventBadgeColor = (event: any): string => { return "var(--color-success-600)"; case "Reopened": return "var(--color-orange-500)"; + case "AckEndedByPageResolve": + return "var(--color-blue-500)"; case "DimensionsUpgraded": return "var(--color-ai-accent)"; case "TitleChanged": @@ -239,6 +243,8 @@ const getEventBadgeText = (event: any): string => { return t("alerts.incidents.timeline.badgeResolved"); case "Reopened": return t("alerts.incidents.timeline.badgeReopened"); + case "AckEndedByPageResolve": + return t("alerts.incidents.timeline.badgeAckEnded"); case "DimensionsUpgraded": return t("alerts.incidents.timeline.badgeDimensionsUpgraded"); case "TitleChanged": @@ -340,6 +346,9 @@ const getInlineEventText = (event: any): string => { ? t("alerts.incidents.timeline.incidentWas") : t("alerts.incidents.timeline.theIncident"); + case "AckEndedByPageResolve": + return t("alerts.incidents.timeline.ackEndedByPageResolve"); + case "Alert": { const alertName = bold(data.alert_name || t("alerts.incidents.timeline.unnamedAlert")); const count = Number(data.count ?? 0); diff --git a/web/src/components/common/KpiCard.vue b/web/src/components/common/KpiCard.vue index ea4304d72d..2fdfa36ed7 100644 --- a/web/src/components/common/KpiCard.vue +++ b/web/src/components/common/KpiCard.vue @@ -30,15 +30,15 @@ along with this program. If not, see . + @@ -380,6 +399,15 @@ along with this program. If not, see . {{ t("workflow.ndv.notInRun") }} + +
+ +
+
.
- +
. v-if="showIo && !nodeMissingFromRun && !runStoredNoData" data-test="workflow-ndv-output" class="relative flex min-w-0 flex-col gap-2" - :class="isReadonlyTrigger ? 'flex-1' : 'w-[var(--io-w,21.25rem)] shrink-0'" + :class="outputSectionClass" :style="isReadonlyTrigger ? undefined : { '--io-w': outputWidth + 'px' }" > @@ -606,6 +649,9 @@ along with this program. If not, see . > {{ t(`workflow.test.stepResult.status.${outputStatus}`) }} + + {{ t("workflow.http.outputCaption") }} +
.
@@ -789,6 +836,24 @@ import OTextarea from "@/lib/forms/Input/OTextarea.vue"; import OTimeCell from "@/lib/core/Table/cells/OTimeCell.vue"; import CodeQueryEditor from "@/components/CodeQueryEditor.vue"; import WorkflowRunSwitcher from "./WorkflowRunSwitcher.vue"; +import WorkflowRunHttpOutcome from "./WorkflowRunHttpOutcome.vue"; +import { DEFAULT_TRIGGER_KIND } from "@/plugins/workflows/triggers"; +import WorkflowRunRequestFacts from "./WorkflowRunRequestFacts.vue"; +import OTag from "@/lib/core/Badge/OTag.vue"; +import type { HttpRequestFacts, WorkflowHistoryRow } from "@/services/workflows"; +import { isHttpRun } from "@/plugins/workflows/httpEndpoint"; +import { + httpPreviewSample, + openHttpPanel, + usesHttpPayload, +} from "@/plugins/workflows/httpEndpointUi"; +import { + RUN_TIME_LONG, + firstFailedStep, + hasStoredRunData, + runReasonOf, +} from "@/plugins/workflows/runStatus"; +import { timestampToTimezoneDate } from "@/utils/zincutils"; import { branchPathCounts, forwardedRunCount, isNoMatchRun } from "./nodeRunOutcome"; import WorkflowTrigger from "@/plugins/workflows/nodes/WorkflowTrigger.vue"; import WorkflowCondition from "@/plugins/workflows/nodes/WorkflowCondition.vue"; @@ -819,6 +884,8 @@ import useWorkflowCanvas, { setNodeEditedInput, getNodeEditedInput, clearNodeEditedInput, + humanizeNodeIds, + persistTestData, } from "@/plugins/workflows/useWorkflowCanvas"; const { t } = useI18nTyped(); @@ -934,6 +1001,79 @@ const canRetryFromHere = computed( () => canvasReadOnly.value && !!historyRunId.value && isRetryableRun(loadedRunRow.value), ); +// ── HTTP request run on the read-only trigger (request facts, failure line) ── +const loadedRunIsHttp = computed(() => !!loadedRunRow.value && isHttpRun(loadedRunRow.value)); +const httpTriggerRecords = computed< + { meta?: { headers?: Record; received_at?: unknown } }[] | null +>(() => { + if (!loadedRunIsHttp.value) return null; + const recs = nodeTestOutput(nodeId.value) || nodeTestInput(nodeId.value); + return recs && recs.length ? recs : null; +}); +const httpFacts = computed( + () => (loadedRunRow.value as WorkflowHistoryRow | null)?.http_request ?? null, +); +const httpHeaders = computed>( + () => httpTriggerRecords.value?.[0]?.meta?.headers ?? {}, +); +const httpReceivedAt = computed( + () => + Number(httpTriggerRecords.value?.[0]?.meta?.received_at) || loadedRunRow.value?._timestamp || 0, +); +const httpNeverRan = computed(() => !hasStoredRunData(loadedRunRow.value)); +const httpFailedStep = computed(() => { + const row = loadedRunRow.value; + const errors = workflowObj.testRun.result?.errors; + if (row?.status !== "failed" || !errors) return null; + const id = firstFailedStep( + errors, + stepTree.value.map((s) => s.id), + ); + const step = id ? stepTree.value.find((s) => s.id === id) : null; + if (!id || !step) return null; + const nodes: { id: string; data?: { node_type?: string } }[] = + workflowObj.currentSelectedWorkflow?.nodes || []; + const node = nodes.find((n) => n.id === id); + const message = errors[id]?.errors?.[0]?.[0]; + return { + step: { id, label: step.label, isFunction: node?.data?.node_type === "function" }, + error: message ? String(humanizeNodeIds(message, t)) : null, + }; +}); +const httpRunOutcome = computed(() => { + const row = loadedRunRow.value; + if (!isReadonlyTrigger.value || !row || !loadedRunIsHttp.value) return null; + return { + status: row.status, + reason: runReasonOf(row), + receivedAt: httpReceivedAt.value, + failedStep: httpFailedStep.value?.step ?? null, + error: httpFailedStep.value?.error ?? null, + triggerKind: currentTriggerKind() ?? DEFAULT_TRIGGER_KIND, + }; +}); +// The request block sits above Output, so the pane scrolls rather than squeezing the event to a few lines. +const outputSectionClass = computed(() => { + if (!isReadonlyTrigger.value) return "w-[var(--io-w,21.25rem)] shrink-0"; + return httpRunOutcome.value ? "flex-1 overflow-y-auto" : "flex-1"; +}); +const canUseRunAsTestInput = computed(() => !!httpRunOutcome.value && !!httpTriggerRecords.value); +// Seeds the same shared state the Test dialog's run picker writes, then hands over to that dialog. +const useRunAsTestInput = () => { + const records = httpTriggerRecords.value; + if (!records) return; + workflowObj.testRun.input = JSON.stringify(records, null, 2); + workflowObj.testRun.inputSource = "run"; + workflowObj.testRun.inputRunLabel = timestampToTimezoneDate( + Math.floor((loadedRunRow.value?.start_time || 0) / 1000), + store.state.timezone, + ); + workflowObj.testRun.fromNode = ""; + persistTestData(); + closeNodeDrawer(); + workflowObj.testRun.show = true; +}; + const retryingFromHere = ref(false); const retryFromHere = async () => { const workflowId = workflowObj.currentSelectedWorkflow?.id; @@ -1082,6 +1222,7 @@ const isTriggerNode = (node: any) => node?.data?.node_type === "workflow_trigger const defaultTriggerSample = computed(() => { const kind = currentTriggerKind(); if (!kind) return []; + if (usesHttpPayload()) return httpPreviewSample(); try { return triggerDef(kind).buildSample() || []; } catch { @@ -1189,6 +1330,8 @@ const outputRecords = computed(() => { // The node's OUTPUT is reported directly by the backend `outputs` map. const recs = nodeTestOutput(nodeId.value); if (recs && recs.length) return recs; + // An HTTP run's trigger emitted the request, so the kind's alert sample would misreport it. + if (httpRunOutcome.value && httpTriggerRecords.value) return httpTriggerRecords.value; // The trigger has no input — its output IS the event. Show its sample even before a // run, so the trigger's Output is always populated (it has no Input pane). if (isTriggerNode(workflowObj.currentSelectedNodeData) && defaultTriggerSample.value.length) @@ -1593,4 +1736,10 @@ const onConfirmDelete = () => { const onOpenChange = async (open: boolean) => { if (!open) await commitAndClose(); }; + +// One overlay per flow: the step dialog closes through its normal path before the panel opens. +const openHttpEndpoint = async () => { + await commitAndClose(); + openHttpPanel(); +}; diff --git a/web/src/components/workflows/WorkflowRunHttpOutcome.spec.ts b/web/src/components/workflows/WorkflowRunHttpOutcome.spec.ts new file mode 100644 index 0000000000..edfd7c2bfc --- /dev/null +++ b/web/src/components/workflows/WorkflowRunHttpOutcome.spec.ts @@ -0,0 +1,119 @@ +// Copyright 2026 OpenObserve Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +import { describe, expect, it } from "vitest"; +import { mount } from "@vue/test-utils"; +import i18n from "@/locales"; +import store from "@/test/unit/helpers/store"; +import WorkflowRunHttpOutcome from "./WorkflowRunHttpOutcome.vue"; + +const mountOutcome = (props: Record) => + mount(WorkflowRunHttpOutcome, { + props: { + status: "failed", + reason: null, + receivedAt: 1000, + failedStep: null, + error: null, + ...props, + }, + global: { + plugins: [i18n, store], + stubs: { + OTimeCell: { + name: "OTimeCell", + props: ["value", "unit", "mode", "format"], + template: "
editor route; back -> list). The canvas / // panel / result drawer are stubbed. -import { vi, describe, it, expect, beforeEach } from "vitest"; +import { vi, describe, it, expect, beforeEach, afterEach } from "vitest"; const { mockRouter, @@ -72,12 +72,17 @@ vi.mock("@/plugins/workflows/useWorkflowCanvas", () => ({ retryWorkflowRun: (...a: any[]) => mockRetry(...a), isRetryableRun: (run: any) => !!run && !!run.error && run.event_type !== "Test" && run.event_type !== "Retry", + currentTriggerKind: () => "alert_fired", + currentTriggerNodeId: () => "t1", })); import { mount, flushPromises } from "@vue/test-utils"; import i18n from "@/locales"; import store from "@/test/unit/helpers/store"; import WorkflowRuns from "./WorkflowRuns.vue"; +import workflowService from "@/services/workflows"; +import { queryClient } from "@/composables/query/queryClient"; +import { httpConfig } from "@/test/unit/helpers/workflowHttp"; const OPageHeaderStub = { name: "OPageHeader", @@ -96,8 +101,8 @@ const OButtonStub = { }; const RunsPanelStub = { name: "WorkflowRunsPanel", - props: ["orgId", "workflowId", "workflowName", "selectedRunId"], - emits: ["select-run"], + props: ["orgId", "workflowId", "workflowName", "selectedRunId", "triggerFilter"], + emits: ["select-run", "update:triggerFilter"], template: `
`, }; const stub = (name: string) => ({ name, template: `
` }); @@ -115,7 +120,27 @@ const globalCfg = { template: `
`, }, WorkflowCanvas: stub("WorkflowCanvas"), - WorkflowTestDialog: stub("WorkflowTestDialog"), + WorkflowTestDialog: { + name: "WorkflowTestDialog", + props: ["seed", "payloadFrom", "seedRunAt"], + template: `
`, + }, + WorkflowHttpEndpointPanel: { + name: "WorkflowHttpEndpointPanel", + props: [ + "open", + "workflowId", + "workflowName", + "kind", + "isDraftOnly", + "workflowEnabled", + "context", + "beforeCreate", + "triggerNodeId", + ], + emits: ["update:open", "run-test"], + template: `
`, + }, }, }; @@ -304,6 +329,19 @@ describe("WorkflowRuns", () => { expect(mockRouter.push).not.toHaveBeenCalled(); }); + it("gives the runs panel one static width whatever the filter", async () => { + const wrapper = await mountRuns(); + const box = () => wrapper.find('[data-test="workflow-runs-panel"]').classes(); + const before = box(); + expect(before).toEqual(expect.arrayContaining(["w-[50rem]", "max-w-[60%]", "max-lg:w-full"])); + panel(wrapper).vm.$emit("update:triggerFilter", "http_request"); + await flushPromises(); + expect(box()).toEqual(before); + panel(wrapper).vm.$emit("update:triggerFilter", "alert_fired"); + await flushPromises(); + expect(box()).toEqual(before); + }); + it("back returns to the workflows list", async () => { const wrapper = await mountRuns(); await wrapper.find(".back-btn").trigger("click"); @@ -373,3 +411,115 @@ describe("WorkflowRuns — retry the loaded run", () => { expect(mockToast).toHaveBeenCalledWith(expect.objectContaining({ variant: "error" })); }); }); + +describe("WorkflowRuns — HTTP endpoint", () => { + const getHttpTrigger = vi.mocked(workflowService.getHttpTrigger); + const button = (w: any) => w.find('[data-test="workflow-runs-http-endpoint"]'); + const chip = (w: any) => w.find('[data-test="workflow-runs-http-endpoint-chip"]'); + const chipVariant = (w: any) => + w + .findAllComponents({ name: "OTag" }) + .find((c: any) => c.find('[data-test="workflow-runs-http-endpoint-chip"]').exists()) + ?.props("variant"); + const httpPanel = (w: any) => w.findComponent({ name: "WorkflowHttpEndpointPanel" }); + + beforeEach(() => { + vi.clearAllMocks(); + queryClient.clear(); + listWorkflows.mockResolvedValue({ data: [] }); + mockLoadRun.mockResolvedValue({ ok: true }); + getHttpTrigger.mockResolvedValue(httpConfig()); + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: true }; + workflowObj.readOnly = false; + workflowObj.testRun = { resultDrawer: { show: false, nodeId: "" }, result: null, show: false }; + workflowObj.currentSelectedWorkflow = { + id: "wf-1", + name: "my flow", + nodes: [], + edges: [], + enabled: true, + }; + }); + afterEach(() => { + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: false }; + }); + + it("shows the button on the read-only page, just before Test, with the live chip", async () => { + const wrapper = await mountRuns(); + expect(workflowObj.readOnly).toBe(true); + expect(button(wrapper).text()).toContain(i18n.global.t("workflow.http.editorButton")); + expect(chipVariant(wrapper)).toBe("success-soft"); + const html = wrapper.html(); + const at = (id: string) => html.indexOf(`data-test="${id}"`); + expect(at("workflow-runs-http-endpoint")).toBeLessThan(at("workflow-runs-test")); + expect(at("workflow-runs-test")).toBeLessThan(at("workflow-runs-edit")); + wrapper.unmount(); + }); + + it("shows the neutral Off chip for an endpoint that is switched off", async () => { + getHttpTrigger.mockResolvedValue(httpConfig({ enabled: false })); + const wrapper = await mountRuns(); + expect(chipVariant(wrapper)).toBe("default-soft"); + expect(chip(wrapper).text()).toBe(i18n.global.t("workflow.http.editorButtonOff")); + wrapper.unmount(); + }); + + it("shows a paused workflow's live endpoint as a warning", async () => { + workflowObj.currentSelectedWorkflow.enabled = false; + const wrapper = await mountRuns(); + expect(chipVariant(wrapper)).toBe("warning-soft"); + wrapper.unmount(); + }); + + it("has no button or panel while HTTP endpoints are disabled for the org", async () => { + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: false }; + // Pages the earlier suites never unmounted fetch when beforeEach turns the flag on. + await flushPromises(); + getHttpTrigger.mockClear(); + const wrapper = await mountRuns(); + expect(button(wrapper).exists()).toBe(false); + expect(httpPanel(wrapper).exists()).toBe(false); + expect(getHttpTrigger).not.toHaveBeenCalled(); + wrapper.unmount(); + }); + + it("opens the panel with this page's workflow in the editor context", async () => { + const wrapper = await mountRuns(); + expect(httpPanel(wrapper).props("open")).toBe(false); + await button(wrapper).trigger("click"); + expect(httpPanel(wrapper).props()).toMatchObject({ + open: true, + workflowId: "wf-1", + workflowName: "my flow", + kind: "alert_fired", + isDraftOnly: false, + workflowEnabled: true, + context: "editor", + triggerNodeId: "t1", + }); + await httpPanel(wrapper).vm.$emit("update:open", false); + expect(httpPanel(wrapper).props("open")).toBe(false); + wrapper.unmount(); + }); + + it("Run test closes the panel, clears the selected run and opens a seeded Test dialog", async () => { + const wrapper = await mountRuns(); + panel(wrapper).vm.$emit("select-run", "run-5"); + await flushPromises(); + expect(panel(wrapper).props("selectedRunId")).toBe("run-5"); + await button(wrapper).trigger("click"); + + const seed = [{ meta: { trigger_source: "http", trigger_id: "test" }, data: [] }]; + await httpPanel(wrapper).vm.$emit("run-test", seed, 42); + await flushPromises(); + + expect(httpPanel(wrapper).props("open")).toBe(false); + expect(panel(wrapper).props("selectedRunId")).toBe(""); + expect(workflowObj.testRun.show).toBe(true); + const dialog = wrapper.findComponent({ name: "WorkflowTestDialog" }); + expect(dialog.props("seed")).toEqual(seed); + expect(dialog.props("payloadFrom")).toBe("http"); + expect(dialog.props("seedRunAt")).toBe(42); + wrapper.unmount(); + }); +}); diff --git a/web/src/components/workflows/WorkflowRuns.vue b/web/src/components/workflows/WorkflowRuns.vue index 823a142768..05ab236f65 100644 --- a/web/src/components/workflows/WorkflowRuns.vue +++ b/web/src/components/workflows/WorkflowRuns.vue @@ -53,6 +53,13 @@ along with this program. If not, see . @@ -166,13 +191,19 @@ import WorkflowCanvas from "@/plugins/workflows/WorkflowCanvas.vue"; import WorkflowNodeDrawer from "./WorkflowNodeDrawer.vue"; import WorkflowTestDialog from "./WorkflowTestDialog.vue"; import WorkflowRunsPanel from "./WorkflowRunsPanel.vue"; +import WorkflowHttpEndpointButton from "./WorkflowHttpEndpointButton.vue"; +import WorkflowHttpEndpointPanel from "./WorkflowHttpEndpointPanel.vue"; import useWorkflowCanvas, { workflowObj, hydrateWorkflow, loadWorkflowRun, isRetryableRun, retryWorkflowRun, + currentTriggerKind, + currentTriggerNodeId, } from "@/plugins/workflows/useWorkflowCanvas"; +import { useWorkflowHttpButton } from "@/plugins/workflows/useWorkflowHttpButton"; +import type { PayloadFrom } from "@/plugins/workflows/httpEndpointUi"; import { workflowsQuery } from "@/services/workflows.queries"; import { queryClient } from "@/composables/query/queryClient"; @@ -181,6 +212,7 @@ const { t } = useI18nTyped(); const { resetWorkflowData } = useWorkflowCanvas(t); const router = useRouter(); + const store = useStore(); const orgId = computed(() => store.state.selectedOrganization.identifier as string); @@ -194,8 +226,31 @@ const activeFolderId = computed( ); const workflowId = computed(() => (router.currentRoute.value.query.id as string) || ""); const workflowName = computed(() => workflowObj.currentSelectedWorkflow?.name || ""); +const isDraft = computed(() => !!workflowObj.currentSelectedWorkflow?.isDraft); +const workflowEnabled = computed(() => workflowObj.currentSelectedWorkflow?.enabled !== false); const selectedRunId = ref(""); +const { + visible: hooksOn, + chip: httpChip, + tooltipKey: httpTooltipKey, +} = useWorkflowHttpButton({ workflowId, isDraft, workflowEnabled }); +const httpKind = computed(() => currentTriggerKind()); +const httpTriggerNodeId = computed(() => currentTriggerNodeId()); +const httpPanelOpen = ref(false); +const testSeed = ref(undefined); +const testPayloadFrom = ref(undefined); +const testSeedRunAt = ref(undefined); +watch( + () => workflowObj.testRun.show, + (show) => { + if (show) return; + testSeed.value = undefined; + testPayloadFrom.value = undefined; + testSeedRunAt.value = undefined; + }, +); + // Steps this run executed that no longer exist in the workflow (deleted/edited // since). Their badges can't render, so the canvas alone would under-report the // run — the banner tells the user the graph has moved on. @@ -243,6 +298,14 @@ const onTest = () => { workflowObj.testRun.show = true; }; +const onHttpRunTest = (seed: unknown[], runAt?: number) => { + httpPanelOpen.value = false; + testSeed.value = seed; + testPayloadFrom.value = "http"; + testSeedRunAt.value = runAt; + onTest(); +}; + // A live test result (from the header Test button, or a step drawer's "Use as Test // Input" which drops the user into a fresh test) replaces the historical run on the // canvas. Deselect the run so the list stops highlighting a row that no longer @@ -362,6 +425,23 @@ const onSelectRun = async (runId: string) => { }); }; +// The panel's "View run" link lands on this same route, which does not remount. +watch( + () => router.currentRoute.value.query.run_id, + (runId) => { + if (typeof runId !== "string" || !runId || runId === selectedRunId.value) return; + httpPanelOpen.value = false; + void onSelectRun(runId); + }, +); + +// A run already in the URL changes no route, so the watch above never reloads it. +const onPanelViewRun = (runId: string) => { + if (runId === router.currentRoute.value.query.run_id && runId !== selectedRunId.value) { + void onSelectRun(runId); + } +}; + onMounted(async () => { // Read-only inspection surface — no editing on the canvas. workflowObj.readOnly = true; diff --git a/web/src/components/workflows/WorkflowRunsPanel.spec.ts b/web/src/components/workflows/WorkflowRunsPanel.spec.ts index 6272fab29b..971b07a6f4 100644 --- a/web/src/components/workflows/WorkflowRunsPanel.spec.ts +++ b/web/src/components/workflows/WorkflowRunsPanel.spec.ts @@ -21,10 +21,12 @@ import { vi, describe, it, expect, beforeEach, afterEach } from "vitest"; -const { mockList, mockToast, mockRetry } = vi.hoisted(() => ({ +const { mockList, mockToast, mockRetry, mockGetHttp, mockAlerts } = vi.hoisted(() => ({ + mockAlerts: vi.fn().mockResolvedValue({ data: { list: [] } }), mockList: vi.fn().mockResolvedValue({ data: [] }), mockToast: vi.fn(), mockRetry: vi.fn().mockResolvedValue({ data: {} }), + mockGetHttp: vi.fn().mockResolvedValue({ configured: false }), })); vi.mock("@/services/workflows", async (importOriginal) => { @@ -33,9 +35,16 @@ vi.mock("@/services/workflows", async (importOriginal) => { default: { getWorkflowHistory: (...a: any[]) => mockList(...a), retryWorkflow: (...a: any[]) => mockRetry(...a), + getHttpTrigger: (...a: any[]) => mockGetHttp(...a), }, }); }); +vi.mock("@/services/alerts", async (importOriginal) => { + const { overlayServiceMock } = await import("@/test/unit/helpers/mockService"); + return overlayServiceMock(await importOriginal(), { + default: { listByFolderId: (...a: any[]) => mockAlerts(...a) }, + }); +}); vi.mock("@/lib/feedback/Toast/useToast", () => ({ toast: (...a: any[]) => mockToast(...a), })); @@ -103,16 +112,17 @@ const globalCfg = { plugins: [i18n, store], stubs: { OTable: OTableStub, - OTimeCell: stub("OTimeCell", ["value", "unit", "mode", "timezone", "emptyLabel"]), + OTimeCell: stub("OTimeCell", ["value", "unit", "mode", "timezone", "emptyLabel", "format"]), OBadge: stub("OBadge", ["variant", "size"]), + OTag: { + name: "OTag", + props: ["type", "value", "size", "label", "variant", "icon"], + template: `{{ label || value }}`, + }, OButton: stub("OButton", ["variant", "size", "iconLeft", "loading"]), OTooltip: stub("OTooltip", ["side", "content"]), DateTime: DateTimeStub, - WorkflowExecutionTimeline: stub("WorkflowExecutionTimeline", [ - "history", - "firingLabel", - "okLabel", - ]), + WorkflowRunsTimeline: stub("WorkflowRunsTimeline", ["runs", "start", "end", "timezone"]), NoData: stub("NoData"), OEmptyState: stub("OEmptyState", ["preset", "filtered"]), }, @@ -284,8 +294,22 @@ describe("test runs are filtered out of run history by default", () => { const toggle = (w: any) => w.find('[data-test="workflow-runs-show-test"]'); const mixed = [ - { run_id: "real", event_type: "AlertFired", start_time: 1, end_time: 2, error: null }, - { run_id: "test", event_type: "Test", start_time: 3, end_time: 4, error: null }, + { + run_id: "real", + event_type: "AlertFired", + start_time: 1, + end_time: 2, + error: null, + status: "succeeded", + }, + { + run_id: "test", + event_type: "Test", + start_time: 3, + end_time: 4, + error: null, + status: "succeeded", + }, ]; beforeEach(() => { @@ -349,6 +373,7 @@ describe("run error text names nodes readably", () => { start_time: 1, end_time: 2, error: "errors in nodes: 2d127ed5-b02a-48ab-8839-10f9d58de3f6", + status: "failed", }; // The reason renders in the status cell's tooltip, so drive that cell and read @@ -398,7 +423,7 @@ describe("run error text names nodes readably", () => { // No error, no tooltip — a successful run must not carry an empty explanation. it("renders no reason tooltip on a run that did not fail", async () => { - expect(await errorText({ ...errRun, error: null })).toBe(""); + expect(await errorText({ ...errRun, error: null, status: "succeeded" })).toBe(""); }); }); @@ -413,6 +438,7 @@ describe("WorkflowRunsPanel — retry row action", () => { end_time: 2000, error: "errors in nodes: abc", event_type: "AlertFired", + status: "failed", ...over, }); @@ -452,7 +478,7 @@ describe("WorkflowRunsPanel — retry row action", () => { }); it("does NOT offer retry on a successful run", async () => { - const w = await rowsFor([run({ error: "" })]); + const w = await rowsFor([run({ error: "", status: "succeeded" })]); expect(w.find('[data-test="workflow-run-retry-r-1"]').exists()).toBe(false); }); @@ -642,13 +668,7 @@ describe("WorkflowRunsPanel — test-run visibility defaults by draft state", () }); }); -// The panel is a fixed 27.5rem (440px) master-detail column, so the column widths -// are a hard budget, not a preference: every pixel over the container pushes a -// column off-screen where no horizontal scrollbar reaches it. -describe("WorkflowRunsPanel — column layout fits the 27.5rem panel", () => { - const PANEL_WIDTH = 440; - const INDEX_COL = 56; - +describe("WorkflowRunsPanel — one fixed column set", () => { const colsOf = (w: any) => w.findComponent(OTableStub as any).props("columns") as any[]; const byId = (w: any, id: string) => colsOf(w).find((c) => c.id === id); @@ -669,50 +689,39 @@ describe("WorkflowRunsPanel — column layout fits the 27.5rem panel", () => { }; }); - // Defect 2/3: the declared widths summed to 918px in a 440px panel, so Duration, - // Started and Ended were pushed out of view entirely. - it("declares a total width that fits the panel", async () => { + it("declares Started, Status, Run, Trigger, Source details, Duration and Actions at their sizes", async () => { const w = await mountCols(); - const total = colsOf(w).reduce((sum, c) => sum + (c.size ?? 0), INDEX_COL); - expect(total).toBeLessThanOrEqual(PANEL_WIDTH); + expect(colsOf(w).map((c) => [c.id, c.size])).toEqual([ + ["start_time", 112], + ["status", 92], + ["run_id", 88], + ["trigger", 124], + ["details", 210], + ["duration", 84], + ["actions", 68], + ]); }); - // Defect 3: a run history is scanned as "when did it run, how long, did it pass". - it("leads with the columns a run history is scanned by", async () => { + it("pins actions last and budgets two icon slots", async () => { const w = await mountCols(); - const order = colsOf(w).map((c) => c.id); - expect(order.slice(0, 3)).toEqual(["start_time", "duration", "status"]); - }); - - // Defect 3: `actions` is pinned right, so declaring it between start_time and - // end_time made the source order lie about the rendered order. - it("declares the pinned actions column last", async () => { - const w = await mountCols(); - const order = colsOf(w).map((c) => c.id); - expect(order[order.length - 1]).toBe("actions"); expect(byId(w, "actions").pinned).toBe("right"); + expect(byId(w, "actions").meta.actionCount).toBe(2); }); - // Defect 2: the Error column flexed to absorb every leftover pixel, so on a - // history with no failures the widest column carried nothing but em-dashes. The - // reason now rides on the Failed badge instead of holding a column open. - it("spends no column width on the error text", async () => { + it("spends no column width on the error text and shows no index column", async () => { const w = await mountCols(); expect(byId(w, "error")).toBeUndefined(); - expect(colsOf(w).some((c) => c.meta?.flex)).toBe(false); + expect(w.findComponent(OTableStub as any).props("showIndex")).toBeFalsy(); }); - // Defect 1: the status cell rendered the Success badge AND the "Test run" badge - // inside a column capped at 100px, clipping the second badge to a stray "T". - it("gives the test-run marker its own column instead of crowding status", async () => { + it("lets Source details absorb what the pinned actions floor leaves", async () => { const w = await mountCols(); - expect(byId(w, "event_type")).toBeTruthy(); + expect(byId(w, "details").meta).toMatchObject({ autoWidth: true, fillRemaining: true }); + for (const c of colsOf(w).filter((c) => !["details", "actions"].includes(c.id))) + expect(c.maxSize).toBe(c.size); }); }); -// Defect 1 (root cause): two badges cannot fit a 100px cell. The status cell must -// render the outcome alone; the test/real distinction is load-bearing and moves to -// its own column rather than being dropped. describe("WorkflowRunsPanel — test-run marker is legible", () => { const testRun = { run_id: "t-1", @@ -750,12 +759,10 @@ describe("WorkflowRunsPanel — test-run marker is legible", () => { template: `
- +
`, }, - OBadge: { name: "OBadge", props: ["variant", "size"], template: `` }, - // The shared stub drops its slot, which would hide the marker it wraps. OTooltip: { name: "OTooltip", props: ["side", "content", "maxWidth"], @@ -773,9 +780,10 @@ describe("WorkflowRunsPanel — test-run marker is legible", () => { expect(w.find(".c-status").text()).not.toContain("Test run"); }); - it("renders the test-run marker in its own cell", async () => { + it("names the test run in its Trigger badge", async () => { const w = await mountRows(); - expect(w.find(".c-type").text()).toContain("Test"); + expect(w.find(".c-trigger").text()).toContain("Test run"); + expect(w.find('[data-test="workflow-run-test-marker"]').exists()).toBe(true); }); }); @@ -788,6 +796,7 @@ describe("WorkflowRunsPanel — a failure still explains itself", () => { end_time: 2000, error: "boom in node abc", event_type: "AlertFired", + status: "failed", }; beforeEach(() => { @@ -832,3 +841,455 @@ describe("WorkflowRunsPanel — a failure still explains itself", () => { expect(String(tip.props("content"))).toContain("boom in node"); }); }); + +// ── Stored statuses and HTTP request runs (AC-WEB-06, AC-UI-10) ──────────────── +describe("WorkflowRunsPanel — stored statuses and HTTP request runs", () => { + const COLUMN_IDS = [ + "start_time", + "status", + "run_id", + "trigger", + "details", + "duration", + "actions", + ]; + const facts = (over: Record = {}) => ({ + credential_prefix: "o2wh_4fK9…oI4k", + caller_ip: "203.0.113.24", + idempotency_key: "deploy-d-4821", + content_type: "application/json", + body_bytes: 68, + status: "failed", + error: null, + ...over, + }); + const row = (over: Record) => ({ + start_time: 1000, + end_time: 2000, + _timestamp: 1000, + error: null, + status: "succeeded", + event_type: "AlertFired", + ...over, + }); + const httpRows = [ + row({ + run_id: "0193f2e8-aaaa", + event_type: "Webhook", + status: "queued", + http_request: facts({ status: "queued", idempotency_key: "deploy-d-4825" }), + }), + row({ + run_id: "0193f25e-bbbb", + event_type: "Webhook", + status: "expired", + http_request: facts({ status: "expired", idempotency_key: null, caller_ip: "198.51.100.7" }), + }), + row({ + run_id: "0193f1f9-cccc", + event_type: "Webhook", + status: "failed", + error: "worker_lost", + http_request: facts({ error: "worker_lost", idempotency_key: "deploy-d-4818" }), + }), + row({ + run_id: "0193f2c4-dddd", + event_type: "Webhook", + status: "failed", + error: "FunctionNode JS error", + http_request: facts({ error: "node_errored" }), + }), + row({ run_id: "alert-1", event_type: "AlertFired" }), + row({ run_id: "retry-1", event_type: "Retry" }), + ]; + + // Renders every declared column's cell slot so each cell is assertable. + const CellTable = { + ...OTableStub, + template: `
+ +
+ + + +
+
`, + }; + const ToggleGroupStub = { + name: "OToggleGroup", + props: ["modelValue", "type", "mobileDropdown"], + emits: ["update:modelValue"], + template: `
`, + }; + const ToggleItemStub = { + name: "OToggleGroupItem", + props: ["value", "size", "iconLeft"], + template: ``, + }; + const SearchStub = { + name: "OSearchInput", + props: ["modelValue", "placeholder", "size", "clearable"], + emits: ["update:modelValue"], + template: ``, + }; + const EmptyStub = { + name: "OEmptyState", + props: ["preset", "title", "description", "actionLabel", "icon", "size", "filtered"], + emits: ["action"], + template: `
{{ title }}
`, + }; + + const mounted: any[] = []; + const mountHttp = async (list: any[] = httpRows) => { + mockList.mockResolvedValue({ data: list }); + const w = mount(WorkflowRunsPanel, { + props: { orgId: "default", workflowId: "wf-http" }, + global: { + ...globalCfg, + stubs: { + ...globalCfg.stubs, + OTable: CellTable, + OToggleGroup: ToggleGroupStub, + OToggleGroupItem: ToggleItemStub, + OSearchInput: SearchStub, + OEmptyState: EmptyStub, + OTooltip: { + name: "OTooltip", + props: ["side", "content", "maxWidth"], + template: ``, + }, + }, + }, + }); + mounted.push(w); + await flushPromises(); + return w; + }; + const colIds = (w: any) => + (w.findComponent(CellTable as any).props("columns") as any[]).map((c) => c.id); + const rowIds = (w: any) => + (w.findComponent(CellTable as any).props("data") as any[]).map((r) => r.run_id); + const setFilter = async (w: any, value: string) => { + w.findComponent(ToggleGroupStub as any).vm.$emit("update:modelValue", value); + await flushPromises(); + }; + const filter = (w: any) => w.find('[data-test="workflow-runs-trigger-filter"]'); + const timeline = (w: any) => w.findComponent({ name: "WorkflowRunsTimeline" }); + + beforeEach(() => { + mockList.mockClear(); + mockRetry.mockClear(); + mockGetHttp.mockReset(); + mockGetHttp.mockResolvedValue({ configured: false }); + mockAlerts.mockReset(); + mockAlerts.mockResolvedValue({ data: { list: [] } }); + (store.state as any).zoConfig.workflow_hooks_enabled = true; + workflowObj.runsHistory = { + list: [], + fetchedAt: 0, + params: { start: 0, end: 0 }, + loading: false, + }; + workflowObj.currentSelectedWorkflow = { + ...workflowObj.currentSelectedWorkflow, + id: "wf-http", + isDraft: false, + http_endpoint_enabled: false, + nodes: [ + { + id: "trig", + data: { node_type: "workflow_trigger", trigger_kind: "alert_fired" }, + }, + ], + }; + }); + + afterEach(() => { + while (mounted.length) mounted.pop().unmount(); + delete (store.state as any).zoConfig.workflow_hooks_enabled; + }); + + it("status comes from the stored status field", async () => { + const w = await mountHttp([row({ run_id: "s-1", status: "succeeded", error: "warning text" })]); + expect(w.find('[data-run="s-1"] .c-status .o-tag').text()).toBe("succeeded"); + }); + + it("an errored row stored as succeeded renders Failed (server-normalised fixture)", async () => { + const w = await mountHttp([row({ run_id: "s-2", status: "failed", error: "boom" })]); + const tag = w.find('[data-run="s-2"] .c-status').findComponent({ name: "OTag" }); + expect(tag.props("type")).toBe("hookRunStatus"); + expect(tag.props("value")).toBe("failed"); + }); + + it("feeds the timeline from the stored status", async () => { + const w = await mountHttp([ + row({ run_id: "t-1", status: "failed", error: null }), + row({ run_id: "t-2", status: "succeeded", error: "x" }), + row({ run_id: "t-3", status: "queued", error: null }), + row({ run_id: "t-5", status: "expired", error: null }), + ]); + const runs = timeline(w).props("runs") as { id: string; status: string }[]; + expect(runs.map((r) => [r.id, r.status])).toEqual([ + ["t-1", "failed"], + ["t-2", "succeeded"], + ["t-3", "queued"], + ["t-5", "expired"], + ]); + }); + + it("pins the timeline to the picked range and keeps it when a filter matches nothing", async () => { + mockGetHttp.mockResolvedValue({ configured: true, enabled: true }); + const w = await mountHttp([ + row({ run_id: "h-1", event_type: "Webhook", http_request: facts() }), + ]); + expect(timeline(w).props("start")).toBe(1000); + expect(timeline(w).props("end")).toBe(2000); + await setFilter(w, "alert_fired"); + expect(rowIds(w)).toEqual([]); + expect(timeline(w).exists()).toBe(true); + expect(timeline(w).props("runs")).toEqual([ + expect.objectContaining({ id: "h-1", matched: false }), + ]); + }); + + it("marks only the filtered rows as matched on the timeline", async () => { + const w = await mountHttp(); + await setFilter(w, "http_request"); + const runs = timeline(w).props("runs") as { id: string; matched: boolean }[]; + expect(runs.filter((r) => r.matched).map((r) => r.id)).toEqual(rowIds(w)); + expect(runs.find((r) => r.id === "alert-1")?.matched).toBe(false); + }); + + it("hides the trigger filter on a workflow that never had an endpoint, same columns", async () => { + const w = await mountHttp([row({ run_id: "alert-1" })]); + expect(filter(w).exists()).toBe(false); + expect(w.find('[data-test="workflow-runs-search"]').exists()).toBe(false); + expect(colIds(w)).toEqual(COLUMN_IDS); + }); + + it("hides the trigger filter while hooks are off on the server", async () => { + delete (store.state as any).zoConfig.workflow_hooks_enabled; + const w = await mountHttp(); + expect(filter(w).exists()).toBe(false); + expect(mockGetHttp.mock.calls.filter((c) => c[0]?.id === "wf-http")).toHaveLength(0); + }); + + it("shows All triggers / the kind / HTTP request once the workflow had HTTP runs", async () => { + const w = await mountHttp(); + expect(filter(w).exists()).toBe(true); + for (const id of ["all", "alert_fired", "http_request"]) + expect(w.find(`[data-test="workflow-runs-trigger-filter-${id}"]`).exists()).toBe(true); + expect(w.find('[data-test="workflow-runs-trigger-filter-alert_fired"]').text()).toBe( + "Alert fired", + ); + }); + + it("shows the trigger filter for an endpoint that exists but has no runs in range", async () => { + mockGetHttp.mockResolvedValue({ configured: true, enabled: false }); + const w = await mountHttp([row({ run_id: "alert-1" })]); + expect(mockGetHttp.mock.calls.some((c) => c[0]?.id === "wf-http")).toBe(true); + expect(filter(w).exists()).toBe(true); + }); + + it("names each run's trigger in words under All triggers", async () => { + const w = await mountHttp(); + expect(w.find('[data-run="0193f2c4-dddd"] .c-trigger').text()).toBe("HTTP request"); + expect(w.find('[data-run="alert-1"] .c-trigger').text()).toBe("Alert fired"); + expect(w.find('[data-run="retry-1"] .c-trigger').text()).toBe("Retry run"); + }); + + it("keeps the identical column set under every filter", async () => { + const w = await mountHttp(); + const sets: unknown[] = []; + for (const f of ["all", "alert_fired", "http_request"]) { + await setFilter(w, f); + sets.push( + (w.findComponent(CellTable as any).props("columns") as any[]).map((c) => [ + c.id, + c.size, + c.maxSize, + ]), + ); + } + expect(colIds(w)).toEqual(COLUMN_IDS); + expect(sets[1]).toEqual(sets[0]); + expect(sets[2]).toEqual(sets[0]); + expect(w.emitted("update:triggerFilter")?.at(-1)).toEqual(["http_request"]); + expect(rowIds(w)).not.toContain("alert-1"); + expect(rowIds(w)).not.toContain("retry-1"); + }); + + it("keeps the same table instance across filters", async () => { + const w = await mountHttp(); + const before = w.findComponent(CellTable as any).vm; + await setFilter(w, "http_request"); + expect(w.findComponent(CellTable as any).vm).toBe(before); + }); + + it("gives every filter option the same fixed width and no icon", async () => { + const w = await mountHttp(); + const items = w.findAllComponents(ToggleItemStub as any); + expect(items).toHaveLength(3); + for (const item of items) { + expect(item.classes()).toEqual(expect.arrayContaining(["w-28", "shrink-0"])); + expect(item.props("iconLeft")).toBeUndefined(); + } + }); + + it("dates Started like the mockups and shows the received time for a queued HTTP run", async () => { + const w = await mountHttp([ + row({ run_id: "alert-1", start_time: 5000 }), + row({ + run_id: "q-1", + event_type: "Webhook", + status: "queued", + start_time: 0, + _timestamp: 7000, + http_request: facts({ status: "queued" }), + }), + ]); + const started = (id: string) => + w.find(`[data-run="${id}"] .c-start_time`).findComponent({ name: "OTimeCell" }); + expect(started("alert-1").props("format")).toBe("MMM d, HH:mm"); + expect(started("alert-1").props("value")).toBe(5000); + expect(started("q-1").props("value")).toBe(7000); + }); + + it("a reason line wraps under its chip instead of cutting off", async () => { + const w = await mountHttp(); + expect(w.find('[data-test="workflow-run-reason-0193f25e-bbbb"]').classes()).toContain( + "whitespace-normal", + ); + }); + + it("a run that never started or has not finished shows no duration", async () => { + const w = await mountHttp(); + for (const id of ["0193f2e8-aaaa", "0193f25e-bbbb"]) + expect(w.find(`[data-run="${id}"] .c-duration`).text()).toBe("—"); + expect(w.find('[data-run="alert-1"] .c-duration').text()).not.toBe("—"); + }); + + it("a kind filter keeps that kind's runs only", async () => { + const w = await mountHttp(); + await setFilter(w, "alert_fired"); + expect(rowIds(w)).toEqual(["alert-1"]); + }); + + it("source details: token prefix over IP and key, the alert name, the retried run, the user", async () => { + mockAlerts.mockResolvedValue({ + data: { list: [{ alert_id: "al-1", name: "checkout_5xx_rate" }] }, + }); + const w = await mountHttp([ + ...httpRows, + row({ run_id: "alert-2", source_id: "al-1" }), + row({ run_id: "alert-3", source_id: "al-gone" }), + row({ run_id: "retry-2", event_type: "Retry", source_id: "3L5y2KdQ-original" }), + row({ run_id: "manual-1", event_type: "Manual", source_id: "root@example.com" }), + ]); + const details = (id: string) => w.find(`[data-test="workflow-run-details-${id}"]`).text(); + expect(details("0193f25e-bbbb")).toContain("o2wh_4fK9…oI4k"); + expect(details("0193f25e-bbbb")).toContain("198.51.100.7 · no idempotency key"); + expect(details("0193f2c4-dddd")).toContain("203.0.113.24 · key deploy-d-4821"); + expect(details("alert-2")).toBe("checkout_5xx_rate"); + expect(details("alert-3")).toBe("al-gone"); + expect(details("retry-2")).toBe("Retry of 3L5y2KdQ"); + expect(details("manual-1")).toBe("root@example.com"); + expect(w.find('[data-run="0193f2c4-dddd"] .c-run_id').text()).toContain("0193f2c4"); + expect(w.find('[data-run="0193f2c4-dddd"] .c-run_id').text()).not.toContain("dddd"); + }); + + it("expired rows show never ran, worker-lost rows say they are not re-run", async () => { + const w = await mountHttp(); + expect(w.find('[data-test="workflow-run-reason-0193f25e-bbbb"]').text()).toBe( + "never ran · queued over 1 h", + ); + expect(w.find('[data-test="workflow-run-reason-0193f1f9-cccc"]').text()).toBe( + "worker lost · not re-run", + ); + expect(w.find('[data-test="workflow-run-reason-0193f2c4-dddd"]').exists()).toBe(false); + }); + + it("every row has an open icon and keeps the retry slot reserved when it cannot retry", async () => { + const w = await mountHttp(); + for (const id of rowIds(w)) { + const actions = w.find(`[data-run="${id}"] .c-actions`); + expect(actions.find(`[data-test="workflow-run-open-${id}"]`).exists()).toBe(true); + const slots = + actions.findAll(`[data-test="workflow-run-retry-${id}"]`).length + + actions.findAll('[data-test="workflow-run-retry-slot"]').length; + expect(slots).toBe(1); + } + await w.find('[data-test="workflow-run-open-alert-1"]').trigger("click"); + expect(w.emitted("select-run")?.at(-1)).toEqual(["alert-1"]); + }); + + it("retry is hidden for never-executed runs", async () => { + const w = await mountHttp(); + expect(w.find('[data-test="workflow-run-retry-0193f25e-bbbb"]').exists()).toBe(false); + expect(w.find('[data-test="workflow-run-retry-0193f1f9-cccc"]').exists()).toBe(false); + expect(w.find('[data-test="workflow-run-retry-0193f2e8-aaaa"]').exists()).toBe(false); + expect(w.find('[data-test="workflow-run-retry-0193f2c4-dddd"]').exists()).toBe(true); + }); + + it("search shows under every filter and matches a run id prefix, key, token prefix or alert", async () => { + mockAlerts.mockResolvedValue({ + data: { list: [{ alert_id: "al-1", name: "checkout_5xx_rate" }] }, + }); + const w = await mountHttp([...httpRows, row({ run_id: "alert-2", source_id: "al-1" })]); + const search = w.findComponent(SearchStub as any); + expect(search.attributes("data-test")).toBe("workflow-runs-search"); + expect(search.props("placeholder")).toBe("Search run ID, idempotency key or alert"); + search.vm.$emit("update:modelValue", "5XX"); + await flushPromises(); + expect(rowIds(w)).toEqual(["alert-2"]); + search.vm.$emit("update:modelValue", "4fK9"); + await flushPromises(); + expect(rowIds(w)).toHaveLength(4); + await setFilter(w, "http_request"); + expect(w.findComponent(SearchStub as any).exists()).toBe(true); + search.vm.$emit("update:modelValue", "DEPLOY-D-4825"); + await flushPromises(); + expect(rowIds(w)).toEqual(["0193f2e8-aaaa"]); + search.vm.$emit("update:modelValue", "0193f2c4"); + await flushPromises(); + expect(rowIds(w)).toEqual(["0193f2c4-dddd"]); + }); + + it("prioritizes search empty and clears search without resetting the trigger filter", async () => { + const w = await mountHttp(httpRows); + await setFilter(w, "http_request"); + w.findComponent(SearchStub).vm.$emit("update:modelValue", "no-such-run"); + await flushPromises(); + const empty = w.findComponent(EmptyStub); + expect(empty.attributes("data-test")).toBe("workflow-runs-search-empty"); + empty.vm.$emit("action"); + await flushPromises(); + expect(w.findComponent(SearchStub).props("modelValue")).toBe(""); + expect(rowIds(w).length).toBeGreaterThan(0); + expect(w.emitted("update:triggerFilter")?.at(-1)).toEqual(["http_request"]); + }); + + it("an empty HTTP filter says so and offers all triggers back", async () => { + mockGetHttp.mockResolvedValue({ configured: true, enabled: true }); + const w = await mountHttp([row({ run_id: "alert-1" })]); + await setFilter(w, "http_request"); + const empty = w.findComponent(EmptyStub as any); + expect(empty.attributes("data-test")).toBe("workflow-runs-http-empty"); + expect(empty.props("title")).toBe("No HTTP requests in this time range"); + expect(empty.props("actionLabel")).toBe("Show all triggers"); + empty.vm.$emit("action"); + await flushPromises(); + expect(w.emitted("update:triggerFilter")?.at(-1)).toEqual(["all"]); + expect(rowIds(w)).toEqual(["alert-1"]); + }); + + it("an empty alert filter names it, says the rest are dimmed and offers all triggers", async () => { + const w = await mountHttp(httpRows.filter((r) => r.event_type === "Webhook")); + await setFilter(w, "alert_fired"); + const empty = w.findComponent(EmptyStub as any); + expect(empty.attributes("data-test")).toBe("workflow-runs-filter-empty"); + expect(empty.props("title")).toBe("No alert-fired runs in this time range"); + expect(empty.props("description")).toBe("The other runs are still on the timeline, dimmed."); + expect(empty.props("actionLabel")).toBe("Show all triggers"); + }); +}); diff --git a/web/src/components/workflows/WorkflowRunsPanel.vue b/web/src/components/workflows/WorkflowRunsPanel.vue index 860d84aada..88318fa042 100644 --- a/web/src/components/workflows/WorkflowRunsPanel.vue +++ b/web/src/components/workflows/WorkflowRunsPanel.vue @@ -80,13 +80,41 @@ along with this program. If not, see .
- - +
+ + + {{ opt.label }} + + +
+ +
+
+ +
@@ -102,17 +130,13 @@ along with this program. If not, see . :loading-reset-key="workflowId" :show-global-filter="false" :default-columns="false" - :show-index="true" :enable-column-resize="true" sort-by="start_time" sort-order="desc" width="100%" - class="h-full w-full" + class="h-full w-full [&_[data-test=o2-table-scroll-container]]:[scrollbar-gutter:stable]" @row-click="openRun" > - @@ -130,63 +177,112 @@ along with this program. If not, see . :value="value" unit="us" mode="absolute" + :format="RUN_TIME_SHORT" :timezone="store.state.timezone" - :empty-label="raw('—')" + :empty-label="dash" /> - - - - - - + + + + + +
@@ -205,22 +301,47 @@ along with this program. If not, see . diff --git a/web/src/components/workflows/WorkflowTestDialog.spec.ts b/web/src/components/workflows/WorkflowTestDialog.spec.ts index b1d7e23382..f68aaf9334 100644 --- a/web/src/components/workflows/WorkflowTestDialog.spec.ts +++ b/web/src/components/workflows/WorkflowTestDialog.spec.ts @@ -18,7 +18,7 @@ // the SAVED workflow through executeTestRun (the /test endpoint is a validator: // it returns per-node ERRORS only). -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { mount, flushPromises } from "@vue/test-utils"; import { nextTick } from "vue"; import i18n from "@/locales"; @@ -46,6 +46,7 @@ vi.mock("@/services/workflows", async (importOriginal) => { import WorkflowTestDialog from "./WorkflowTestDialog.vue"; import { workflowObj, LAST_TEST_RUN } from "@/plugins/workflows/useWorkflowCanvas"; import { buildTestSampleText } from "@/plugins/workflows/testSample"; +import { INCIDENT_EVENT_TYPES } from "@/plugins/workflows/incidentSample"; // ── stubs ──────────────────────────────────────────────────────────────────── const ODrawerStub = { @@ -757,3 +758,104 @@ describe("destination suppression (P0 safety)", () => { expect(mockTestWorkflow.mock.calls[0][0].suppress_destinations).toBe(false); }); }); + +describe("WorkflowTestDialog Payload from", () => { + const payloadSelect = (w: any) => + w + .findAllComponents(OSelectStub as any) + .find((s: any) => s.attributes("data-test") === "workflow-test-payload-from"); + + beforeEach(() => { + vi.clearAllMocks(); + resetState(); + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: true }; + }); + afterEach(() => { + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: false }; + }); + + it("Payload from offers the kind samples and HTTP request", async () => { + const w = mountDialog(); + const select = payloadSelect(w); + expect(select.props("options").map((o: any) => o.value)).toEqual(["trigger", "http"]); + await select.vm.$emit("update:modelValue", "http"); + const [{ meta }] = JSON.parse(workflowObj.testRun.input); + expect(meta).toMatchObject({ trigger_source: "http", trigger_id: "test" }); + expect(workflowObj.testRun.inputSource).toBe("sample"); + await select.vm.$emit("update:modelValue", "trigger"); + expect(JSON.parse(workflowObj.testRun.input)[0].meta.trigger_source).toBeUndefined(); + }); + + it("HTTP request posts to /workflows/test with the in-memory graph", async () => { + mockTestWorkflow.mockResolvedValue({ data: { errors: {} } }); + const w = mountDialog(); + await payloadSelect(w).vm.$emit("update:modelValue", "http"); + await primary(w).trigger("click"); + await flushPromises(); + expect(mockTestWorkflow).toHaveBeenCalled(); + const body = JSON.stringify(mockTestWorkflow.mock.calls[0]); + expect(body).toContain("trigger_source"); + expect(body).toContain("f1"); + }); + + it("seed prefills the input", () => { + const seed = [{ meta: { trigger_source: "http", trigger_id: "test" }, data: [{ k: 1 }] }]; + mount(WorkflowTestDialog, { props: { seed, payloadFrom: "http" }, global: globalConfig }); + expect(JSON.parse(workflowObj.testRun.input)).toEqual(seed); + }); + + it("a seed from the latest HTTP run is labelled as that run, not as a sample", () => { + const seed = [{ meta: { trigger_source: "http", trigger_id: "t9" }, data: [{ k: 1 }] }]; + mount(WorkflowTestDialog, { + props: { seed, payloadFrom: "http", seedRunAt: 1_700_000_000_000_000 }, + global: globalConfig, + }); + expect(workflowObj.testRun.inputSource).toBe("run"); + expect(workflowObj.testRun.inputRunLabel).not.toBe(""); + }); + + it("an Incident Event workflow offers each event's sample and HTTP request", async () => { + setWorkflow( + [ + { id: "t1", data: { node_type: "workflow_trigger", trigger_kind: "incident_event" } }, + destNode, + ], + [{ source: "t1", target: "d1" }], + ); + const w = mountDialog(); + const values = payloadSelect(w) + .props("options") + .map((o: any) => o.value); + expect(values).toEqual([...INCIDENT_EVENT_TYPES, "http"]); + await payloadSelect(w).vm.$emit("update:modelValue", "resolved"); + expect(JSON.parse(workflowObj.testRun.input)[0].meta.event_type).toBe("resolved"); + }); + + it("an Incident Event workflow opens on the variant its saved input holds", () => { + setWorkflow( + [ + { id: "t1", data: { node_type: "workflow_trigger", trigger_kind: "incident_event" } }, + destNode, + ], + [{ source: "t1", target: "d1" }], + ); + workflowObj.testRun.input = JSON.stringify([{ meta: { event_type: "resolved" }, data: [] }]); + const w = mountDialog(); + expect(payloadSelect(w).props("modelValue")).toBe("resolved"); + }); + + it("an HTTP run used as test input selects HTTP request", () => { + workflowObj.testRun.input = JSON.stringify([ + { meta: { trigger_source: "http", trigger_id: "t9" }, data: [] }, + ]); + workflowObj.testRun.inputSource = "run"; + const w = mountDialog(); + expect(payloadSelect(w).props("modelValue")).toBe("http"); + }); + + it("offers no HTTP payload when hooks are off", () => { + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: false }; + const w = mountDialog(); + expect(payloadSelect(w)).toBeUndefined(); + }); +}); diff --git a/web/src/components/workflows/WorkflowTestDialog.vue b/web/src/components/workflows/WorkflowTestDialog.vue index 0d8377424a..e53d864a94 100644 --- a/web/src/components/workflows/WorkflowTestDialog.vue +++ b/web/src/components/workflows/WorkflowTestDialog.vue @@ -75,6 +75,16 @@ along with this program. If not, see .
+
+ + {{ t("workflow.test.payloadFromHint") }} +
+
@@ -155,9 +165,13 @@ import OTag from "@/lib/core/Badge/OTag.vue"; import OBanner from "@/lib/feedback/Banner/OBanner.vue"; import CodeQueryEditor from "@/components/CodeQueryEditor.vue"; import { toast } from "@/lib/feedback/Toast/useToast"; +import { workflowHooksEnabled } from "@/plugins/workflows/httpEndpoint"; +import { httpPreviewSample, type PayloadFrom } from "@/plugins/workflows/httpEndpointUi"; +import { isHttpRequestInput } from "@/plugins/workflows/httpRequestSample"; import { workflowObj, + triggerDef, nodeMeta, executeTestRun, flowOrderedNodeIds, @@ -173,9 +187,50 @@ import { persistTestData, } from "@/plugins/workflows/useWorkflowCanvas"; +const props = defineProps<{ + /** Prefills the input (latest HTTP run, or the HTTP sample from the endpoint panel). */ + seed?: unknown[]; + payloadFrom?: PayloadFrom; + /** Received time (µs) of the HTTP run `seed` came from; unset for a generated sample. */ + seedRunAt?: number; +}>(); + const { t } = useI18nTyped(); const store = useStore(); +const hooksOn = computed(() => workflowHooksEnabled(store.state.zoConfig)); +// "trigger" stands for a kind without variants; a kind with variants offers each variant key. +const HTTP_CHOICE: PayloadFrom = "http"; +const KIND_CHOICE: PayloadFrom = "trigger"; +const kindVariants = triggerDef(currentTriggerKind()).sampleVariants ?? []; +const firstKindChoice = kindVariants[0]?.key ?? KIND_CHOICE; +const savedVariant = (): string | undefined => { + try { + const event = JSON.parse(workflowObj.testRun.input || "[]")?.[0]?.meta?.event_type; + return kindVariants.find((v) => v.key === event)?.key; + } catch { + return undefined; + } +}; +const startsAsHttp = + props.payloadFrom === HTTP_CHOICE || + (!props.payloadFrom && isHttpRequestInput(workflowObj.testRun.input)); +const payloadChoice = ref(startsAsHttp ? HTTP_CHOICE : (savedVariant() ?? firstKindChoice)); +const payloadFromOptions = computed(() => { + const kindOptions = kindVariants.length + ? kindVariants.map((v) => ({ label: v.labelKey ? t(v.labelKey) : raw(v.key), value: v.key })) + : [{ label: t(triggerDef(currentTriggerKind()).labelKey), value: KIND_CHOICE }]; + return [...kindOptions, { label: t("workflow.http.payloadHttp"), value: HTTP_CHOICE }]; +}); +// Every Test runs the in-memory graph; only the seeded payload changes. +const payloadSelection = computed({ + get: () => payloadChoice.value, + set: (v) => { + payloadChoice.value = v; + seedSample(); + }, +}); + const running = ref(false); // v-model:open — closing the dialog (X / overlay) clears the flag. const open = computed({ @@ -188,10 +243,14 @@ const edges = computed(() => workflowObj.currentSelectedWorkflow?.edges | // Seed the sample payload the first time the dialog opens (persisted after). onMounted(() => { - if (!workflowObj.testRun.input) { - workflowObj.testRun.input = buildTriggerSampleText(currentTriggerKind()); - workflowObj.testRun.inputSource = "sample"; - workflowObj.testRun.inputRunLabel = ""; + if (props.seed?.length) { + workflowObj.testRun.input = JSON.stringify(props.seed, null, 2); + workflowObj.testRun.inputSource = props.seedRunAt ? "run" : "sample"; + workflowObj.testRun.inputRunLabel = props.seedRunAt + ? timestampToTimezoneDate(Math.floor(props.seedRunAt / 1000), store.state.timezone) + : ""; + } else if (!workflowObj.testRun.input) { + seedSample(); } // State persisted before this flag existed carries no value; pin it ON so the // safe default is explicit rather than inferred from undefined. @@ -337,13 +396,21 @@ const run = async () => { }); }; -const seedSample = () => { - workflowObj.testRun.input = buildTriggerSampleText(currentTriggerKind()); +function seedSample() { + const variant = kindVariants.find((v) => v.key === payloadChoice.value); + if (hooksOn.value && payloadChoice.value === HTTP_CHOICE) { + workflowObj.testRun.input = JSON.stringify(httpPreviewSample("test"), null, 2); + } else if (variant) { + workflowObj.testRun.input = JSON.stringify(variant.build(), null, 2); + } else { + workflowObj.testRun.input = buildTriggerSampleText(currentTriggerKind()); + } workflowObj.testRun.inputSource = "sample"; workflowObj.testRun.inputRunLabel = ""; -}; +} const resetSample = () => seedSample(); + const revertInput = () => seedSample(); // The editor re-emits its own value on mount/format, so only a real text change diff --git a/web/src/components/workflows/WorkflowsList.spec.ts b/web/src/components/workflows/WorkflowsList.spec.ts index 605ce30dd6..0b0b3b5e38 100644 --- a/web/src/components/workflows/WorkflowsList.spec.ts +++ b/web/src/components/workflows/WorkflowsList.spec.ts @@ -55,6 +55,7 @@ vi.mock("@/services/workflows", async (importOriginal) => { listWorkflows: vi.fn(), deleteWorkflow: vi.fn(), enableWorkflow: vi.fn(), + getHttpTrigger: vi.fn(), }, }); }); @@ -77,6 +78,31 @@ vi.mock("@/components/workflows/WorkflowView.vue", () => ({ }, })); +vi.mock("@/components/workflows/WorkflowHttpEndpointPanel.vue", () => ({ + default: { + name: "WorkflowHttpEndpointPanel", + props: [ + "open", + "workflowId", + "workflowName", + "kind", + "isDraftOnly", + "workflowEnabled", + "context", + ], + emits: ["update:open"], + template: '
', + }, +})); + +vi.mock("@/utils/clipboard", () => ({ + copyToClipboard: vi.fn(async () => true), + copyPendingText: vi.fn(async (text: Promise) => { + await text; + return true; + }), +})); + import { mount, flushPromises } from "@vue/test-utils"; import { nextTick } from "vue"; import i18n from "@/locales"; @@ -84,6 +110,8 @@ import store from "@/test/unit/helpers/store"; import WorkflowsList from "./WorkflowsList.vue"; import ConfirmDialog from "@/components/ConfirmDialog.vue"; import workflowService from "@/services/workflows"; +import { queryClient } from "@/composables/query/queryClient"; +import { copyPendingText } from "@/utils/clipboard"; const t = (k: string, v?: any) => i18n.global.t(k, v ?? {}); const listWorkflows = workflowService.listWorkflows as any; @@ -468,6 +496,14 @@ describe("WorkflowsList", () => { ).toEqual(["#", "name", "description", "trigger", "updated_at", "actions"]); }); + // Measured in the built UI: the Incident Event tag 94 + gap 4 + HTTP tag 63 + 16 cell padding. + it("sizes Trigger for an Incident Event tag and the HTTP tag side by side", async () => { + wrapper = mountList(); + await flushPromises(); + const trigger = (table(wrapper).props("columns") as any[]).find((c) => c.id === "trigger"); + expect(trigger.size).toBeGreaterThanOrEqual(177); + }); + it("marks the actions column as an action column", async () => { wrapper = mountList(); await flushPromises(); @@ -1188,3 +1224,96 @@ describe("WorkflowsList", () => { }); }); }); + +describe("WorkflowsList HTTP endpoint", () => { + let wrapper: any = null; + const getHttpTrigger = (workflowService as any).getHttpTrigger; + const panel = (w: any) => w.findComponent({ name: "WorkflowHttpEndpointPanel" }); + + beforeEach(() => { + vi.clearAllMocks(); + resetListViewState(); + queryClient.clear(); + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: true }; + }); + afterEach(() => { + wrapper?.unmount(); + wrapper = null; + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: false }; + }); + + const load = async (list: any[]) => { + listWorkflows.mockResolvedValue({ data: list }); + wrapper = mountList(); + await flushPromises(); + return wrapper; + }; + + it("shows the HTTP tag on any kind with an endpoint", async () => { + const incident = { + id: "t1", + data: { node_type: "workflow_trigger", trigger_kind: "incident_event" }, + }; + const w = await load([ + makeWorkflow(1, { http_endpoint_enabled: true }), + makeWorkflow(2, { http_endpoint_enabled: true, nodes: [incident] }), + makeWorkflow(3, { http_endpoint_enabled: true, is_draft: true }), + makeWorkflow(4), + ]); + expect(w.findAll('[data-test="workflow-list-http-tag"]')).toHaveLength(3); + }); + + it("has no HTTP tab", async () => { + const w = await load([makeWorkflow(1, { http_endpoint_enabled: true })]); + expect(w.find('[data-test="workflow-list-tab-http_request"]').exists()).toBe(false); + }); + + it("HTTP endpoint… opens the panel on any row", async () => { + const w = await load([makeWorkflow(1), makeWorkflow(2, { is_draft: true, enabled: false })]); + await w.find('[data-test="workflow-list-workflow-2-http-endpoint"]').trigger("click"); + expect(panel(w).props()).toMatchObject({ + open: true, + workflowId: "wf-2", + workflowName: "workflow-2", + isDraftOnly: true, + workflowEnabled: false, + context: "list", + }); + }); + + it("Copy endpoint URL fetches the http_trigger query and copies its url", async () => { + getHttpTrigger.mockResolvedValue({ configured: true, url: "https://o2/h/1" }); + const w = await load([makeWorkflow(1, { http_endpoint_enabled: true }), makeWorkflow(2)]); + expect(w.find('[data-test="workflow-list-workflow-2-copy-endpoint"]').exists()).toBe(false); + await w.find('[data-test="workflow-list-workflow-1-copy-endpoint"]').trigger("click"); + await flushPromises(); + expect(getHttpTrigger).toHaveBeenCalledWith({ org_identifier: expect.any(String), id: "wf-1" }); + await expect(vi.mocked(copyPendingText).mock.calls[0][0]).resolves.toBe("https://o2/h/1"); + expect(mockToast).toHaveBeenCalledWith( + expect.objectContaining({ message: t("toastMessages.workflows.endpointUrlCopied") }), + ); + }); + + it("Copy endpoint URL toasts an error when the clipboard refuses the write", async () => { + getHttpTrigger.mockResolvedValue({ configured: true, url: "https://o2/h/1" }); + vi.mocked(copyPendingText).mockImplementationOnce(async (text) => { + await text; + return false; + }); + const w = await load([makeWorkflow(1, { http_endpoint_enabled: true })]); + await w.find('[data-test="workflow-list-workflow-1-copy-endpoint"]').trigger("click"); + await flushPromises(); + expect(mockToast).toHaveBeenCalledWith( + expect.objectContaining({ variant: "error", message: t("common.copyError") }), + ); + }); + + it("shows no HTTP tag, HTTP endpoint item or Copy endpoint URL when hooks are off", async () => { + store.state.zoConfig = { ...store.state.zoConfig, workflow_hooks_enabled: false }; + const w = await load([makeWorkflow(1, { http_endpoint_enabled: true })]); + expect(w.find('[data-test="workflow-list-http-tag"]').exists()).toBe(false); + expect(w.find('[data-test="workflow-list-workflow-1-http-endpoint"]').exists()).toBe(false); + expect(w.find('[data-test="workflow-list-workflow-1-copy-endpoint"]').exists()).toBe(false); + expect(panel(w).exists()).toBe(false); + }); +}); diff --git a/web/src/components/workflows/WorkflowsList.vue b/web/src/components/workflows/WorkflowsList.vue index d48ff35edc..54618f3f7d 100644 --- a/web/src/components/workflows/WorkflowsList.vue +++ b/web/src/components/workflows/WorkflowsList.vue @@ -191,12 +191,23 @@ along with this program. If not, see . diff --git a/web/src/enterprise/components/AIObservability/AiNoAgentIdentityBanner.vue b/web/src/enterprise/components/AIObservability/AiNoAgentIdentityBanner.vue new file mode 100644 index 0000000000..1eaf802221 --- /dev/null +++ b/web/src/enterprise/components/AIObservability/AiNoAgentIdentityBanner.vue @@ -0,0 +1,70 @@ + + + + + + diff --git a/web/src/enterprise/components/AIObservability/AiScopeBar.vue b/web/src/enterprise/components/AIObservability/AiScopeBar.vue index 348fba99ee..7908d2df05 100644 --- a/web/src/enterprise/components/AIObservability/AiScopeBar.vue +++ b/web/src/enterprise/components/AIObservability/AiScopeBar.vue @@ -244,6 +244,8 @@ const emit = defineEmits<{ (e: "update:selectedVersion", value: string): void; (e: "filter-mode-change", value: FilterMode): void; (e: "stream-change", value: string): void; + /** The user picked an env, agent or version: the agent scope is now an explicit choice. */ + (e: "agent-pick"): void; }>(); // v-model bridge for the stream picker — read the prop, write both the model @@ -276,13 +278,16 @@ const onStreamChange = (value: unknown) => { // useAgentScope selection refs and re-derives the lower lists). const onSelectedEnvChange = (value: string) => { emit("update:selectedEnv", value); + emit("agent-pick"); }; const onSelectedAgentNameChange = (value: string) => { emit("update:selectedAgentName", value); + emit("agent-pick"); }; const onSelectedVersionChange = (value: string) => { emit("update:selectedVersion", value); + emit("agent-pick"); }; diff --git a/web/src/enterprise/components/AIObservability/ExperimentBrowser.vue b/web/src/enterprise/components/AIObservability/ExperimentBrowser.vue index eda6d58498..91b534e27e 100644 --- a/web/src/enterprise/components/AIObservability/ExperimentBrowser.vue +++ b/web/src/enterprise/components/AIObservability/ExperimentBrowser.vue @@ -334,6 +334,7 @@ import { useMutation } from "@tanstack/vue-query"; import llmExperimentsService, { type LlmExperiment } from "@/services/llm-experiments.service"; import { comparisonEligibility, + experimentCostText, experimentEvidence, groupExperiments, type ComparisonIneligibilityReason, @@ -627,8 +628,8 @@ function translateComparisonReason(reason: ComparisonIneligibilityReason) { } function costLabel(experiment: LlmExperiment) { - const cost = evidence(experiment).cost; - return cost === null ? raw("—") : raw(`$${cost.toFixed(4)}`); + const { cost, costIncomplete } = evidence(experiment); + return experimentCostText(cost, costIncomplete, t); } function isBaseline(experiment: LlmExperiment) { diff --git a/web/src/enterprise/components/AIObservability/ExperimentComparisonRowDrawer.spec.ts b/web/src/enterprise/components/AIObservability/ExperimentComparisonRowDrawer.spec.ts index 05994f6b2b..911f274494 100644 --- a/web/src/enterprise/components/AIObservability/ExperimentComparisonRowDrawer.spec.ts +++ b/web/src/enterprise/components/AIObservability/ExperimentComparisonRowDrawer.spec.ts @@ -23,7 +23,7 @@ import ExperimentComparisonRowDrawer from "./ExperimentComparisonRowDrawer.vue"; function detail(experimentId: string, output: string, score: number): ExperimentRowDetail { return { experimentId, - snapshot: { datasetId: "dataset", datasetVersion: 1 }, + snapshot: { datasetId: "dataset", datasetVersion: 1, datasetWriteVersion: 1 }, navigation: { rowIndex: 0, totalRows: 1, diff --git a/web/src/enterprise/components/AIObservability/ExperimentForm.drafts.spec.ts b/web/src/enterprise/components/AIObservability/ExperimentForm.drafts.spec.ts index 07517676ae..87398f37af 100644 --- a/web/src/enterprise/components/AIObservability/ExperimentForm.drafts.spec.ts +++ b/web/src/enterprise/components/AIObservability/ExperimentForm.drafts.spec.ts @@ -49,7 +49,10 @@ vi.mock("@/services/llm-datasets.service", async () => ({ default: { list: vi.fn(), listItems: vi.fn() }, })); vi.mock("@/services/online-evals.service", () => ({ - default: { scorers: { list: vi.fn() }, providers: { list: vi.fn() } }, + default: { + scorers: { list: vi.fn() }, + providers: { list: vi.fn(), testConfig: vi.fn() }, + }, })); vi.mock("@/services/llm-prompts.service", () => ({ default: { list: vi.fn(), resolve: vi.fn() }, @@ -176,6 +179,7 @@ describe("ExperimentForm drafts", () => { } as never); vi.mocked(onlineEvalsService.scorers.list).mockResolvedValue([scorer] as never); vi.mocked(onlineEvalsService.providers.list).mockResolvedValue([provider] as never); + vi.mocked(onlineEvalsService.providers.testConfig).mockResolvedValue("ok" as never); vi.mocked(llmExperimentsService.preview).mockResolvedValue(previewResult as never); vi.mocked(llmExperimentsService.create).mockResolvedValue({ experiment: { id: "exp-1" }, diff --git a/web/src/enterprise/components/AIObservability/ExperimentForm.spec.ts b/web/src/enterprise/components/AIObservability/ExperimentForm.spec.ts index 2057009487..77314a889e 100644 --- a/web/src/enterprise/components/AIObservability/ExperimentForm.spec.ts +++ b/web/src/enterprise/components/AIObservability/ExperimentForm.spec.ts @@ -51,7 +51,10 @@ vi.mock("@/services/llm-datasets.service", async () => ({ default: { list: vi.fn(), listItems: vi.fn() }, })); vi.mock("@/services/online-evals.service", () => ({ - default: { scorers: { list: vi.fn() }, providers: { list: vi.fn() } }, + default: { + scorers: { list: vi.fn() }, + providers: { list: vi.fn(), testConfig: vi.fn().mockResolvedValue("ok") }, + }, })); vi.mock("@/services/llm-prompts.service", () => ({ default: { list: vi.fn(), resolve: vi.fn() }, @@ -68,7 +71,10 @@ const dataset = { id: "ds-1", name: "RAG regression set", description: null, + // 128 single-item writes and one batch: the row counter and the shown + // version differ, and only the row counter is a valid pin. globalVersion: 12, + writeVersion: 4, itemCount: 128, tags: [], sources: { trace: 0, annotation: 0, manual: 0 }, @@ -225,6 +231,20 @@ describe("ExperimentForm", () => { expect(payload).not.toHaveProperty("userPrompt"); }); + it("pins the dataset's row version but shows its write version", async () => { + wrapper = await createWrapper(); + fillValid(wrapper); + await flushPromises(); + + const meta = wrapper.find('[data-test="ai-experiment-form-dataset-meta"]').text(); + expect(meta).toContain("v4"); + expect(meta).not.toContain("v12"); + + await submit(wrapper); + const [, payload] = vi.mocked(llmExperimentsService.create).mock.calls[0]; + expect(payload.datasetVersion).toBe(12); + }); + it("omits the system message when it is blank", async () => { wrapper = await createWrapper(); fillValid(wrapper); diff --git a/web/src/enterprise/components/AIObservability/ExperimentForm.vue b/web/src/enterprise/components/AIObservability/ExperimentForm.vue index d1ff61fad5..22154674c6 100644 --- a/web/src/enterprise/components/AIObservability/ExperimentForm.vue +++ b/web/src/enterprise/components/AIObservability/ExperimentForm.vue @@ -113,7 +113,7 @@ > {{ t("aiObservability.experiments.form.datasetPinned", { - version: selectedDatasetVersion, + version: shownDatasetVersion, }) }} {{ @@ -533,7 +533,7 @@ > {{ t("aiObservability.experiments.scorerApplicability", { - scorer: entry.scorerId, + scorer: scorerName(entry.scorerId), version: entry.scorerVersion, eligible: entry.eligibleSlotCount, noReference: entry.noReferenceSlotCount, @@ -562,6 +562,24 @@