diff --git a/.github/dependabot.yml b/.github/dependabot.yml index f041fd4..a227392 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -18,7 +18,11 @@ updates: schedule: interval: "weekly" day: "monday" - open-pull-requests-limit: 3 + open-pull-requests-limit: 1 + groups: + github-actions: + patterns: + - "*" labels: - "dependencies" - "ci" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6026b94..d2ea104 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,23 +14,12 @@ on: - '*.md' - '.github/workflows/docs.yml' -# Cancel superseded runs for the same PR so a new push does not leave the -# previous run occupying a shared GitHub-hosted runner slot. Pushes to main -# fall back to the unique run_id, giving every main run its own group so it is -# never cancelled — not while executing (cancel-in-progress is false off PRs) -# and not while queued (a shared group drops an older pending run when a newer -# one lands, regardless of cancel-in-progress), so the full matrix and the -# coverage-badge commit job always run to completion. The group is prefixed -# with github.workflow because concurrency groups are repo-wide, not scoped to -# a single workflow. +# Keep the default gate small and cancel superseded work on both PRs and main. +# Expensive engine, sandbox, Windows and release checks live in extended-ci.yml. concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true -# Default the workflow token to read-only. Jobs that execute PR-supplied -# code (build/test, e2e, lint) must never be able to push to the repo, even -# on same-repository PRs where GITHUB_TOKEN would otherwise be writable. -# Only the dedicated badge-commit job below opts back into contents: write. permissions: contents: read @@ -38,23 +27,13 @@ jobs: build: name: Build & Test runs-on: self-hosted - strategy: - # Surface failures on every OS independently instead of cancelling the - # whole matrix when one runner fails. - fail-fast: false - matrix: - # Windows runners sit in a smaller, longer-queuing pool. PRs run - # Linux-only to keep the shared-runner queue short; the full matrix - # (including windows-latest) still runs on pushes to main. - os: ${{ github.event_name == 'pull_request' && fromJSON('["ubuntu-latest"]') || fromJSON('["ubuntu-latest", "windows-latest"]') }} - go-version: ["1.25.x"] - + timeout-minutes: 10 steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: - go-version: ${{ matrix.go-version }} + go-version: "1.25.x" cache: true - name: Verify dependencies @@ -63,107 +42,22 @@ jobs: - name: Build run: go build ./... - # Force bash on Windows runners too: pwsh's legacy native-argument - # passing splits `-coverprofile=coverage.out` and feeds `.out` to go - # as a package import path, producing `FAIL .out [setup failed]`. - # Git Bash is preinstalled on windows-latest and parses args verbatim. - - name: Test (with race detector and coverage) - shell: bash + - name: Test run: go test -race -timeout 120s -covermode=atomic -coverpkg=./... -coverprofile=coverage.out ./... - # Self-hosted coverage badge: parse the total from `go tool cover -func` - # and rewrite .github/badges/coverage.json. The shields.io endpoint badge - # in README.md reads this JSON via raw.githubusercontent.com. - # Ubuntu-only: the Windows leg of the matrix uses a different shell - # toolchain and we only need one canonical coverage number. - - name: Compute coverage and update badge - id: coverage - if: matrix.os == 'ubuntu-latest' + - name: Report coverage run: | pct=$(go tool cover -func=coverage.out | awk '/^total:/ {gsub("%","",$3); print $3}') if [ -z "$pct" ]; then echo "could not parse total coverage" >&2 exit 1 fi - color=$(awk -v p="$pct" 'BEGIN { - if (p+0 >= 80) print "brightgreen"; - else if (p+0 >= 60) print "yellow"; - else print "red"; - }') - mkdir -p .github/badges - printf '{\n "schemaVersion": 1,\n "label": "coverage",\n "message": "%s%%",\n "color": "%s"\n}\n' \ - "$pct" "$color" > .github/badges/coverage.json - echo "pct=$pct" >> "$GITHUB_OUTPUT" - echo "color=$color" >> "$GITHUB_OUTPUT" - echo "::notice::Total coverage: ${pct}% (${color})" - - # Hand the badge to the push-only update-coverage-badge job below via - # an artifact. PRs (including forks) still run the compute step above - # for visibility, but skip the upload so they can never trigger a push. - # - # include-hidden-files: true is required — the badge lives under - # .github/, and actions/upload-artifact defaults to excluding files - # whose path contains any dot-prefixed segment. Without this the - # upload would silently match nothing and, combined with - # if-no-files-found: error, fail the build job on every push to main. - - name: Upload coverage badge artifact - if: matrix.os == 'ubuntu-latest' && github.event_name == 'push' && github.ref == 'refs/heads/main' - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 - with: - name: coverage-badge - path: .github/badges/coverage.json - if-no-files-found: error - include-hidden-files: true - retention-days: 1 + echo "::notice::Total coverage: ${pct}%" - update-coverage-badge: - name: Commit coverage badge - needs: build - # Gate the entire job — not just individual steps — on push-to-main so - # the contents: write token below is never issued for pull_request runs. - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + e2e-smoke: + name: E2E Smoke runs-on: self-hosted - permissions: - contents: write - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - ref: badges - - - name: Download coverage badge artifact - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 - with: - name: coverage-badge - path: .github/badges/ - - - name: Commit updated coverage badge - run: | - if git diff --quiet -- .github/badges/coverage.json; then - echo "coverage badge unchanged, nothing to commit" - exit 0 - fi - pct=$(awk -F'"' '/"message"/ {print $4}' .github/badges/coverage.json) - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add .github/badges/coverage.json - git commit -m "chore(ci): update coverage badge to ${pct} [skip ci]" - git push origin badges - - e2e-windows: - # Windows e2e is intentionally narrower than the Linux e2e: it does not - # set SKILL_UP_FULL_E2E, so the LLM-dependent tests in e2e/cli_test.go, - # e2e/agent_test.go and e2e/mcp_test.go self-skip. The mock-engine and - # script-judge contract tests still exercise the Windows-specific code - # paths added by issue #31 — shell selection, MSYS argv handling, - # QuoteWindows, NewShellCmd's WaitDelay, the script-judge interpreter - # dispatch — through the full CLI pipeline. Real-LLM coverage stays on - # the Linux e2e job below. - name: E2E (none runtime, Windows) - # Skip on PRs to avoid the longer-queuing Windows runner pool; the - # Windows-specific code paths are still exercised on every push to main. - if: github.event_name != 'pull_request' - runs-on: self-hosted - timeout-minutes: 20 + timeout-minutes: 15 steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -172,11 +66,8 @@ jobs: go-version: "1.25.x" cache: true - # Match the Build & Test step's shell choice so pwsh's legacy argv - # passing does not split `-coverprofile`-style flags. - - name: Run e2e tests (none runtime, quick mode) - shell: bash - run: go test -tags e2e -timeout 1200s -count=1 -v ./e2e + - name: Run quick e2e smoke + run: go test -tags e2e -timeout 1200s -count=1 -v -run 'Test(Pipeline|CLI|Contract|NoneRuntime)_' ./e2e env: SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-artifacts @@ -184,342 +75,15 @@ jobs: if: always() && hashFiles('e2e-artifacts/**') != '' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: e2e-windows-workspaces - path: e2e-artifacts/ - if-no-files-found: ignore - retention-days: 14 - - e2e: - name: E2E (none runtime) - runs-on: self-hosted - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - # Fork PRs run without access to repository secrets. Detect that early - # and skip the rest of the job so we don't burn CI minutes installing - # CLIs only to have every LLM-dependent test hit a 401. - - name: Check secret availability - id: secrets - env: - DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - run: | - if [ -z "$DASHSCOPE_API_KEY" ]; then - echo "::notice::Skipping e2e: DASHSCOPE_API_KEY not provisioned (likely a fork PR)." - echo "available=false" >> "$GITHUB_OUTPUT" - else - echo "available=true" >> "$GITHUB_OUTPUT" - fi - - - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 - if: steps.secrets.outputs.available == 'true' - with: - go-version: "1.25.x" - cache: true - - - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 - if: steps.secrets.outputs.available == 'true' - with: - node-version: "22" - - - name: Install claude-code CLI - if: steps.secrets.outputs.available == 'true' - run: | - npm install -g --include=optional @anthropic-ai/claude-code - claude --version - - - name: Install codex CLI - if: steps.secrets.outputs.available == 'true' - run: | - # Pin to the version declared in internal/agent/codex.go (codexDefaultVersion). - npm install -g --include=optional '@openai/codex@0.80.0' - codex --version - - - name: Install qwen-code CLI - if: steps.secrets.outputs.available == 'true' - run: | - npm install -g @qwen-code/qwen-code - qwen --version - - - name: Install qodercli (pinned v1.0.14) - if: steps.secrets.outputs.available == 'true' - env: - QODER_VERSION: "1.0.14" - QODER_SHA256: "4d83ec3d3a0948b73012cfdd6e2757be0a5da5c89a288425a5607fd42be1de7b" - run: | - TARBALL=$(mktemp) - curl -fsSL "https://qoder-ide.oss-accelerate.aliyuncs.com/qodercli/releases/${QODER_VERSION}/qodercli-linux-x64.tar.gz" -o "$TARBALL" - echo "$QODER_SHA256 $TARBALL" | sha256sum -c - - mkdir -p "$HOME/.qoder/bin" - tar -xzf "$TARBALL" -C "$HOME/.qoder/bin" - rm -f "$TARBALL" - echo "$HOME/.qoder/bin" >> "$GITHUB_PATH" - - - name: Verify qodercli on PATH - if: steps.secrets.outputs.available == 'true' - run: | - which qodercli - qodercli --version || true - - - name: Run e2e tests (none runtime, full mode) - if: steps.secrets.outputs.available == 'true' - # SKILL_UP_FULL_E2E unlocks tests that drive a real model. DashScope's - # Anthropic- and OpenAI-compatible endpoints back claude-code and codex - # respectively; QODER_PERSONAL_ACCESS_TOKEN authenticates qodercli. - # SKILL_UP_E2E_ARTIFACT_DIR tells e2e tests to copy their workspace - # (stdout.json / response.md / transcript / result.json …) to a stable - # path before t.TempDir cleanup, so the upload-artifact step below can - # surface them for post-mortem. - run: go test -tags e2e -timeout 1800s -count=1 -v ./e2e - env: - SKILL_UP_FULL_E2E: "1" - SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-artifacts - DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - # claude_code + dashscope provider → routed through the Anthropic-compat endpoint. - DASHSCOPE_BASE_URL: https://dashscope.aliyuncs.com/apps/anthropic - # Pin every anthropic-provider hop to qwen3.6-plus so no e2e path - # accidentally calls real Claude, regardless of which model the - # eval.yaml (or any hard-coded fallback elsewhere in the codebase) - # would otherwise pick. - ANTHROPIC_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - ANTHROPIC_BASE_URL: https://dashscope.aliyuncs.com/apps/anthropic - ANTHROPIC_MODEL: qwen3.6-plus - # Same idea for openai-provider hops (codex test pins "gpt-5.4" in - # its eval.yaml; OPENAI_MODEL overrides that to qwen3.6-plus). - OPENAI_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - OPENAI_BASE_URL: https://dashscope.aliyuncs.com/compatible-mode/v1 - OPENAI_MODEL: qwen3.6-plus - DASHSCOPE_MODEL: qwen3.6-plus - QODER_PERSONAL_ACCESS_TOKEN: ${{ secrets.QODER_ACCESS_TOKEN }} - - - name: Upload e2e workspace artifacts - if: always() && steps.secrets.outputs.available == 'true' && hashFiles('e2e-artifacts/**') != '' - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 - with: - name: e2e-workspaces + name: e2e-smoke-workspaces path: e2e-artifacts/ if-no-files-found: ignore retention-days: 14 - e2e-opensandbox: - name: E2E (opensandbox runtime) - runs-on: self-hosted - # Pulling the execd + sandbox images and bootstrapping the agent inside the - # sandbox is slower than the none-runtime job; give it generous headroom. - timeout-minutes: 35 - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - # The opensandbox runtime is self-hosted on the runner (see below), so no - # OPENSANDBOX_* secret is needed — but the agent under test still calls a - # real model, which needs DASHSCOPE_API_KEY. Fork PRs lack it: skip early. - - name: Check secret availability - id: secrets - env: - DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - run: | - if [ -z "$DASHSCOPE_API_KEY" ]; then - echo "::notice::Skipping opensandbox e2e: DASHSCOPE_API_KEY not provisioned (likely a fork PR)." - echo "available=false" >> "$GITHUB_OUTPUT" - else - echo "available=true" >> "$GITHUB_OUTPUT" - fi - - - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 - if: steps.secrets.outputs.available == 'true' - with: - go-version: "1.25.x" - cache: true - - - name: Install uv - if: steps.secrets.outputs.available == 'true' - uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 - - - name: Install OpenSandbox server - if: steps.secrets.outputs.available == 'true' - run: | - # Pin the server version: it must stay compatible with the execd - # image pinned in the next step, and an unpinned "latest" would let - # an unrelated future release silently change behavior or break CI. - uv tool install 'opensandbox-server==0.1.13' - # uv tool install drops console scripts under ~/.local/bin. - echo "$HOME/.local/bin" >> "$GITHUB_PATH" - - - name: Start OpenSandbox server - if: steps.secrets.outputs.available == 'true' - env: - # Image the sandbox containers boot from. node:22 is a full Debian - # image that already ships node 22 + npm + curl + git, so skill-up's - # in-sandbox agent bootstrap is a near no-op. Bare ubuntu:latest does - # not work — it lacks curl/git/node and the bootstrap fails. - OPENSANDBOX_IMAGE: node:22 - # execd is the bootstrap binary OpenSandbox injects into every - # sandbox. Pinned together with opensandbox-server 0.1.13 above; - # bump both in lockstep if the server log reports an execd - # compatibility error. - OPENSANDBOX_EXECD_IMAGE: opensandbox/execd:v1.0.16 - run: | - docker version - api_key="ci-$(openssl rand -hex 16)" - config="$RUNNER_TEMP/sandbox.toml" - # network_mode = "host" lets the natively-running server and the - # sandbox containers it spawns all share the runner's loopback, so - # the e2e process reaches both without bridge endpoint rewriting. - cat > "$config" < "$log" 2>&1 & - for i in $(seq 1 60); do - if curl -fsS http://127.0.0.1:8080/health 2>/dev/null | grep -q healthy; then - echo "OpenSandbox server is healthy" - break - fi - if [ "$i" -eq 60 ]; then - echo "::error::OpenSandbox server did not become healthy within 120s" - cat "$log" - exit 1 - fi - sleep 2 - done - { - echo "OPENSANDBOX_API_KEY=$api_key" - echo "OPENSANDBOX_BASE_URL=http://127.0.0.1:8080" - echo "OPENSANDBOX_IMAGE=$OPENSANDBOX_IMAGE" - } >> "$GITHUB_ENV" - - - name: Run e2e tests (opensandbox runtime) - if: steps.secrets.outputs.available == 'true' - # Exercise the codex agent here: claude_code already has real-model - # coverage in the none-runtime job, whereas codex is otherwise only - # tested against a fake binary. This run is its sole real coverage. - run: go test -tags e2e -timeout 1800s -count=1 -v -run TestAgent_Codex_OpenSandboxRuntime ./e2e - env: - # codex speaks the OpenAI wire API → supply its key, endpoint and - # model via OPENAI_* (DashScope's OpenAI-compatible endpoint, - # authenticated with the DashScope key). - OPENAI_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - OPENAI_BASE_URL: https://dashscope.aliyuncs.com/compatible-mode/v1 - OPENAI_MODEL: qwen3.6-plus - # Copies the in-sandbox agent workspace out before t.TempDir cleanup - # so the upload step below can surface it for post-mortem. - SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-opensandbox-artifacts - - - name: Upload OpenSandbox server log - if: always() && steps.secrets.outputs.available == 'true' - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 - with: - name: opensandbox-server-log - path: ${{ runner.temp }}/opensandbox-server.log - if-no-files-found: ignore - retention-days: 14 - - - name: Upload opensandbox e2e workspace artifacts - if: always() && steps.secrets.outputs.available == 'true' && hashFiles('e2e-opensandbox-artifacts/**') != '' - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 - with: - name: e2e-opensandbox-workspaces - path: e2e-opensandbox-artifacts/ - if-no-files-found: ignore - retention-days: 14 - - e2e-docker: - name: E2E (docker runtime) - runs-on: self-hosted - # Container lifecycle + image pull on a clean runner; comfortably under - # 15 min in practice but leave headroom for Docker Hub rate-limit retries. - timeout-minutes: 20 - # No API keys / LLM credentials needed: these tests drive the docker - # runtime against a real daemon directly, no agent or model in the loop. - # That also means they run on fork PRs, unlike the other two e2e jobs. - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 - with: - go-version: "1.25.x" - cache: true - - - name: Verify docker is available - # ubuntu-latest runners ship docker pre-installed and running; this - # makes the dependency explicit so the failure mode is obvious if - # GitHub ever changes the image. - run: | - docker version - docker info - - - name: Pre-pull base image - # Pre-pulling outside the test binary keeps the per-test timeouts - # tight and surfaces registry / rate-limit issues as their own step - # rather than a misleading test failure. - run: docker pull alpine:3.20 - - - name: Run docker integration tests - # `docker_integration` is the build tag added in internal/runtime/ - # docker_integration_test.go. Tests skip cleanly if the daemon is - # missing, so this command is also safe to run locally. - run: go test -tags docker_integration -timeout 600s -count=1 -v ./internal/runtime/ - - e2e-docker-full: - name: E2E (docker runtime, full LLM) - runs-on: self-hosted - timeout-minutes: 25 - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - - - name: Check secret availability - id: secrets - env: - DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - run: | - if [ -z "$DASHSCOPE_API_KEY" ]; then - echo "::notice::Skipping docker full e2e: DASHSCOPE_API_KEY not provisioned (likely a fork PR)." - echo "available=false" >> "$GITHUB_OUTPUT" - else - echo "available=true" >> "$GITHUB_OUTPUT" - fi - - - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 - if: steps.secrets.outputs.available == 'true' - with: - go-version: "1.25.x" - cache: true - - - name: Pre-pull Docker image - if: steps.secrets.outputs.available == 'true' - run: docker pull node:22 - - - name: Run docker full e2e - if: steps.secrets.outputs.available == 'true' - run: go test -tags e2e -timeout 1800s -count=1 -v -run TestAgent_ClaudeCode_DockerRuntime ./e2e - env: - SKILL_UP_FULL_E2E: "1" - SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-docker-full-artifacts - ANTHROPIC_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} - ANTHROPIC_BASE_URL: https://dashscope.aliyuncs.com/apps/anthropic - ANTHROPIC_MODEL: qwen3.6-plus - - - name: Upload docker full e2e artifacts - if: always() && steps.secrets.outputs.available == 'true' && hashFiles('e2e-docker-full-artifacts/**') != '' - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 - with: - name: e2e-docker-full-workspaces - path: e2e-docker-full-artifacts/ - if-no-files-found: ignore - retention-days: 14 - lint: name: Lint runs-on: self-hosted + timeout-minutes: 10 steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -530,9 +94,10 @@ jobs: - name: Check formatting run: | - if [ "$(gofmt -l . | wc -l)" -gt 0 ]; then + files=$(gofmt -l .) + if [ -n "$files" ]; then echo "Following files are not formatted:" - gofmt -l . + echo "$files" exit 1 fi @@ -546,30 +111,3 @@ jobs: - name: revive run: revive -config revive.toml ./... - - release-dryrun: - name: GoReleaser Check - runs-on: self-hosted - steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - with: - fetch-depth: 0 - - - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 - with: - go-version: "1.25.x" - cache: true - - - name: GoReleaser check - uses: goreleaser/goreleaser-action@5daf1e915a5f0af01ddbcd89a43b8061ff4f1a89 # v7 - with: - distribution: goreleaser - version: "~> v2" - args: check - - - name: GoReleaser snapshot build - uses: goreleaser/goreleaser-action@5daf1e915a5f0af01ddbcd89a43b8061ff4f1a89 # v7 - with: - distribution: goreleaser - version: "~> v2" - args: release --snapshot --clean --skip=publish diff --git a/.github/workflows/extended-ci.yml b/.github/workflows/extended-ci.yml new file mode 100644 index 0000000..62066d6 --- /dev/null +++ b/.github/workflows/extended-ci.yml @@ -0,0 +1,337 @@ +name: Extended CI + +on: + merge_group: + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + e2e-windows: + name: E2E (none runtime, Windows) + runs-on: self-hosted + timeout-minutes: 20 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version: "1.25.x" + cache: true + + - name: Run e2e tests (none runtime, quick mode) + shell: bash + run: go test -tags e2e -timeout 1200s -count=1 -v ./e2e + env: + SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-artifacts + + - name: Upload e2e workspace artifacts + if: always() && hashFiles('e2e-artifacts/**') != '' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: e2e-windows-workspaces + path: e2e-artifacts/ + if-no-files-found: ignore + retention-days: 14 + + e2e: + name: E2E (none runtime) + runs-on: self-hosted + timeout-minutes: 35 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Check secret availability + id: secrets + env: + DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + run: | + if [ -z "$DASHSCOPE_API_KEY" ]; then + echo "::notice::Skipping e2e: DASHSCOPE_API_KEY not provisioned." + echo "available=false" >> "$GITHUB_OUTPUT" + else + echo "available=true" >> "$GITHUB_OUTPUT" + fi + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + if: steps.secrets.outputs.available == 'true' + with: + go-version: "1.25.x" + cache: true + + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6 + if: steps.secrets.outputs.available == 'true' + with: + node-version: "22" + + - name: Install claude-code CLI + if: steps.secrets.outputs.available == 'true' + run: | + npm install -g --include=optional @anthropic-ai/claude-code + claude --version + + - name: Install codex CLI + if: steps.secrets.outputs.available == 'true' + run: | + npm install -g --include=optional '@openai/codex@0.80.0' + codex --version + + - name: Install qwen-code CLI + if: steps.secrets.outputs.available == 'true' + run: | + npm install -g @qwen-code/qwen-code + qwen --version + + - name: Install qodercli (pinned v1.0.14) + if: steps.secrets.outputs.available == 'true' + env: + QODER_VERSION: "1.0.14" + QODER_SHA256: "4d83ec3d3a0948b73012cfdd6e2757be0a5da5c89a288425a5607fd42be1de7b" + run: | + TARBALL=$(mktemp) + curl -fsSL "https://qoder-ide.oss-accelerate.aliyuncs.com/qodercli/releases/${QODER_VERSION}/qodercli-linux-x64.tar.gz" -o "$TARBALL" + echo "$QODER_SHA256 $TARBALL" | sha256sum -c - + mkdir -p "$HOME/.qoder/bin" + tar -xzf "$TARBALL" -C "$HOME/.qoder/bin" + rm -f "$TARBALL" + echo "$HOME/.qoder/bin" >> "$GITHUB_PATH" + + - name: Verify qodercli on PATH + if: steps.secrets.outputs.available == 'true' + run: | + which qodercli + qodercli --version || true + + - name: Run e2e tests (none runtime, full mode) + if: steps.secrets.outputs.available == 'true' + run: go test -tags e2e -timeout 1800s -count=1 -v ./e2e + env: + SKILL_UP_FULL_E2E: "1" + SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-artifacts + DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + DASHSCOPE_BASE_URL: https://dashscope.aliyuncs.com/apps/anthropic + ANTHROPIC_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + ANTHROPIC_BASE_URL: https://dashscope.aliyuncs.com/apps/anthropic + ANTHROPIC_MODEL: qwen3.6-plus + OPENAI_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + OPENAI_BASE_URL: https://dashscope.aliyuncs.com/compatible-mode/v1 + OPENAI_MODEL: qwen3.6-plus + DASHSCOPE_MODEL: qwen3.6-plus + QODER_PERSONAL_ACCESS_TOKEN: ${{ secrets.QODER_ACCESS_TOKEN }} + + - name: Upload e2e workspace artifacts + if: always() && steps.secrets.outputs.available == 'true' && hashFiles('e2e-artifacts/**') != '' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: e2e-workspaces + path: e2e-artifacts/ + if-no-files-found: ignore + retention-days: 14 + + e2e-opensandbox: + name: E2E (opensandbox runtime) + runs-on: self-hosted + timeout-minutes: 35 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Check secret availability + id: secrets + env: + DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + run: | + if [ -z "$DASHSCOPE_API_KEY" ]; then + echo "::notice::Skipping opensandbox e2e: DASHSCOPE_API_KEY not provisioned." + echo "available=false" >> "$GITHUB_OUTPUT" + else + echo "available=true" >> "$GITHUB_OUTPUT" + fi + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + if: steps.secrets.outputs.available == 'true' + with: + go-version: "1.25.x" + cache: true + + - name: Install uv + if: steps.secrets.outputs.available == 'true' + uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + + - name: Install OpenSandbox server + if: steps.secrets.outputs.available == 'true' + run: | + uv tool install 'opensandbox-server==0.1.13' + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + + - name: Start OpenSandbox server + if: steps.secrets.outputs.available == 'true' + env: + OPENSANDBOX_IMAGE: node:22 + OPENSANDBOX_EXECD_IMAGE: opensandbox/execd:v1.0.16 + run: | + docker version + api_key="ci-$(openssl rand -hex 16)" + config="$RUNNER_TEMP/sandbox.toml" + cat > "$config" < "$log" 2>&1 & + for i in $(seq 1 60); do + if curl -fsS http://127.0.0.1:8080/health 2>/dev/null | grep -q healthy; then + echo "OpenSandbox server is healthy" + break + fi + if [ "$i" -eq 60 ]; then + echo "::error::OpenSandbox server did not become healthy within 120s" + cat "$log" + exit 1 + fi + sleep 2 + done + { + echo "OPENSANDBOX_API_KEY=$api_key" + echo "OPENSANDBOX_BASE_URL=http://127.0.0.1:8080" + echo "OPENSANDBOX_IMAGE=$OPENSANDBOX_IMAGE" + } >> "$GITHUB_ENV" + + - name: Run e2e tests (opensandbox runtime) + if: steps.secrets.outputs.available == 'true' + run: go test -tags e2e -timeout 1800s -count=1 -v -run TestAgent_Codex_OpenSandboxRuntime ./e2e + env: + OPENAI_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + OPENAI_BASE_URL: https://dashscope.aliyuncs.com/compatible-mode/v1 + OPENAI_MODEL: qwen3.6-plus + SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-opensandbox-artifacts + + - name: Upload OpenSandbox server log + if: always() && steps.secrets.outputs.available == 'true' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: opensandbox-server-log + path: ${{ runner.temp }}/opensandbox-server.log + if-no-files-found: ignore + retention-days: 14 + + - name: Upload opensandbox e2e workspace artifacts + if: always() && steps.secrets.outputs.available == 'true' && hashFiles('e2e-opensandbox-artifacts/**') != '' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: e2e-opensandbox-workspaces + path: e2e-opensandbox-artifacts/ + if-no-files-found: ignore + retention-days: 14 + + e2e-docker: + name: E2E (docker runtime) + runs-on: self-hosted + timeout-minutes: 20 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version: "1.25.x" + cache: true + + - name: Verify docker is available + run: | + docker version + docker info + + - name: Pre-pull base image + run: docker pull alpine:3.20 + + - name: Run docker integration tests + run: go test -tags docker_integration -timeout 600s -count=1 -v ./internal/runtime/ + + e2e-docker-full: + name: E2E (docker runtime, full LLM) + runs-on: self-hosted + timeout-minutes: 25 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Check secret availability + id: secrets + env: + DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + run: | + if [ -z "$DASHSCOPE_API_KEY" ]; then + echo "::notice::Skipping docker full e2e: DASHSCOPE_API_KEY not provisioned." + echo "available=false" >> "$GITHUB_OUTPUT" + else + echo "available=true" >> "$GITHUB_OUTPUT" + fi + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + if: steps.secrets.outputs.available == 'true' + with: + go-version: "1.25.x" + cache: true + + - name: Pre-pull Docker image + if: steps.secrets.outputs.available == 'true' + run: docker pull node:22 + + - name: Run docker full e2e + if: steps.secrets.outputs.available == 'true' + run: go test -tags e2e -timeout 1800s -count=1 -v -run TestAgent_ClaudeCode_DockerRuntime ./e2e + env: + SKILL_UP_FULL_E2E: "1" + SKILL_UP_E2E_ARTIFACT_DIR: ${{ github.workspace }}/e2e-docker-full-artifacts + ANTHROPIC_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} + ANTHROPIC_BASE_URL: https://dashscope.aliyuncs.com/apps/anthropic + ANTHROPIC_MODEL: qwen3.6-plus + + - name: Upload docker full e2e artifacts + if: always() && steps.secrets.outputs.available == 'true' && hashFiles('e2e-docker-full-artifacts/**') != '' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: e2e-docker-full-workspaces + path: e2e-docker-full-artifacts/ + if-no-files-found: ignore + retention-days: 14 + + release-dryrun: + name: GoReleaser Check + runs-on: self-hosted + timeout-minutes: 15 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version: "1.25.x" + cache: true + + - name: GoReleaser check + uses: goreleaser/goreleaser-action@5daf1e915a5f0af01ddbcd89a43b8061ff4f1a89 # v7 + with: + distribution: goreleaser + version: "~> v2" + args: check + + - name: GoReleaser snapshot build + uses: goreleaser/goreleaser-action@5daf1e915a5f0af01ddbcd89a43b8061ff4f1a89 # v7 + with: + distribution: goreleaser + version: "~> v2" + args: release --snapshot --clean --skip=publish diff --git a/.github/workflows/build-runner-image.yml b/.github/workflows/runner-image.yml similarity index 78% rename from .github/workflows/build-runner-image.yml rename to .github/workflows/runner-image.yml index c7216a1..7b5fc6b 100644 --- a/.github/workflows/build-runner-image.yml +++ b/.github/workflows/runner-image.yml @@ -1,13 +1,13 @@ -name: Build & push skill-eval runner image +name: Runner Image # Builds the runner image used by the Agent Skill Eval action (action.yml): # bakes skill-up + claude + codex + qodercli from public registries and pushes # to ghcr.io//skill-up-runner via the built-in GITHUB_TOKEN (no PAT). # # Triggers: -# - workflow_call: invoked by skill-eval-self.yml so the dogfood eval always +# - workflow_call: invoked by skill-upper-self-eval.yml so the dogfood eval always # runs against an image built from the same commit (no pull/push race). -# - push to main touching the action/image -> republish. +# - push to main touching the action runtime -> republish. # - workflow_dispatch -> manual (re)publish at an explicit tag. # # Fork PRs lack package-write on GITHUB_TOKEN: they build without pushing. @@ -23,11 +23,16 @@ on: required: false type: string default: 'v0.1.0' + publish_latest: + description: 'Also publish the latest tag' + required: false + type: boolean + default: false push: branches: [main] paths: + - 'action.yml' - 'action/**' - - '.github/workflows/build-runner-image.yml' workflow_dispatch: inputs: tag: @@ -35,10 +40,19 @@ on: required: true type: string default: 'v0.1.0' + publish_latest: + description: 'Also publish the latest tag' + required: true + type: boolean + default: true env: IMAGE: ghcr.io/${{ github.repository_owner }}/skill-up-runner +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build: runs-on: self-hosted @@ -53,6 +67,7 @@ jobs: env: # Fork PRs can't push packages; build-only for them. SAME_REPO: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} + PUBLISH_LATEST: ${{ github.event_name == 'push' || inputs.publish_latest }} run: | { echo "tags<<__TAGS__" @@ -63,9 +78,12 @@ jobs: # not be able to replace the production runner image. echo "${IMAGE}:pr-${{ github.event.pull_request.number }}" else - # push to main / manual dispatch = release: publish the tag + latest. + # push to main / manual dispatch = release; reusable callers can + # opt into latest explicitly, but self-eval uses an isolated tag. echo "${IMAGE}:${{ inputs.tag || 'v0.1.0' }}" - echo "${IMAGE}:latest" + if [ "$PUBLISH_LATEST" = "true" ]; then + echo "${IMAGE}:latest" + fi fi echo "__TAGS__" } >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/skill-eval-self.yml b/.github/workflows/skill-upper-self-eval.yml similarity index 81% rename from .github/workflows/skill-eval-self.yml rename to .github/workflows/skill-upper-self-eval.yml index ff41303..4b91419 100644 --- a/.github/workflows/skill-eval-self.yml +++ b/.github/workflows/skill-upper-self-eval.yml @@ -1,4 +1,4 @@ -name: Self-eval (dogfood the action) +name: Skill Upper Self-Eval # Dogfoods the Agent Skill Eval action (action.yml) against this repo's own # skill-upper skill — once per engine (claude_code / codex / qodercli) — so a @@ -17,32 +17,15 @@ name: Self-eval (dogfood the action) # dashscope's qwen3.6-plus — same as the repo's e2e jobs — so no path ever # calls real Claude / real GPT, regardless of engine. -# Triggered only when the action or the evaluated skill changes: on PRs (pre- -# merge gate) and on main (post-merge regression), same paths filter. +# Manual only. This workflow runs live agent evals and builds a runner image +# before the matrix, so it is intentionally kept out of PR/main CI. on: - pull_request: - paths: - - 'action.yml' - - 'action/**' - - 'skills/skill-upper/**' - - '.github/workflows/skill-eval-self.yml' - - '.github/workflows/build-runner-image.yml' - push: - branches: [main] - # Keep in sync with the pull_request paths above (GitHub workflow YAML - # does not support anchors). - paths: - - 'action.yml' - - 'action/**' - - 'skills/skill-upper/**' - - '.github/workflows/skill-eval-self.yml' - - '.github/workflows/build-runner-image.yml' workflow_dispatch: # LLM evals are expensive: a superseding push to the same PR cancels the # in-flight round instead of running both to completion. concurrency: - group: self-eval-${{ github.event.pull_request.number || github.ref }} + group: self-eval-${{ github.ref }} cancel-in-progress: true # Least-privilege default for every job (the image job overrides with @@ -55,7 +38,10 @@ jobs: # jobs below always eval with the action code + engines baked at the same ref # (otherwise they'd race the parallel image build and pull a stale :v0.1.0). image: - uses: ./.github/workflows/build-runner-image.yml + uses: ./.github/workflows/runner-image.yml + with: + tag: self-eval-${{ github.run_id }} + publish_latest: false permissions: contents: read packages: write @@ -85,13 +71,12 @@ jobs: steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - # Point the (local) action at the image the `image` job just built for THIS - # run: a PR-scoped tag on PRs, the release tag on main. This keeps the - # dogfood testing the PR's own image while never consuming or clobbering - # the mutable v0.1.0 that released users pull via alibaba/skill-up@v0.1.0. + # Point the local action at the image the `image` job just built for this + # run. The tag is intentionally run-scoped so manual dogfood runs from + # feature branches never clobber production runner image tags. - name: Pin action to this run's runner image run: | - tag="${{ github.event_name == 'pull_request' && format('pr-{0}', github.event.pull_request.number) || 'v0.1.0' }}" + tag="self-eval-${{ github.run_id }}" img="ghcr.io/${{ github.repository_owner }}/skill-up-runner:${tag}" sed -i "s#image: docker://ghcr.io/[^[:space:]]*/skill-up-runner:.*#image: docker://${img}#" action.yml echo "dogfood will use ${img}" @@ -105,14 +90,22 @@ jobs: env: # Reference each secret by literal name (no dynamic secrets[...] # indexing, which CodeQL flags as exposing the whole secrets context). - ENGINE_KEY: ${{ matrix.engine == 'qodercli' && secrets.QODER_ACCESS_TOKEN || secrets.DASHSCOPE_API_KEY }} + QODER_ACCESS_TOKEN: ${{ secrets.QODER_ACCESS_TOKEN }} DASHSCOPE_API_KEY: ${{ secrets.DASHSCOPE_API_KEY }} run: | - if [ -z "$ENGINE_KEY" ] || [ -z "$DASHSCOPE_API_KEY" ]; then + if [ -z "$DASHSCOPE_API_KEY" ]; then + echo "::notice::Skipping self-eval (${{ matrix.engine }}): required secret not provisioned (likely a fork PR)." + echo "available=false" >> "$GITHUB_OUTPUT" + elif [ "${{ matrix.engine }}" = "qodercli" ] && [ -z "$QODER_ACCESS_TOKEN" ]; then echo "::notice::Skipping self-eval (${{ matrix.engine }}): required secret not provisioned (likely a fork PR)." echo "available=false" >> "$GITHUB_OUTPUT" else echo "available=true" >> "$GITHUB_OUTPUT" + if [ "${{ matrix.engine }}" = "qodercli" ]; then + echo "api_key=${QODER_ACCESS_TOKEN}" >> "$GITHUB_OUTPUT" + else + echo "api_key=${DASHSCOPE_API_KEY}" >> "$GITHUB_OUTPUT" + fi fi - name: Run skill-upper eval via the action @@ -139,7 +132,7 @@ jobs: engine: ${{ matrix.engine }} provider: ${{ matrix.provider }} model: ${{ matrix.model }} - api-key: ${{ matrix.engine == 'qodercli' && secrets.QODER_ACCESS_TOKEN || secrets.DASHSCOPE_API_KEY }} + api-key: ${{ steps.secrets.outputs.api_key }} # Run cases concurrently so the larger per-case timeout (600s, needed # by the slower codex coder model) doesn't blow up wall-clock — but # keep qodercli serial: concurrent qodercli processes race on the fixed @@ -150,11 +143,15 @@ jobs: # resolves cases (evals/cases/*.yaml) relative to the skill root. skill-target: skills/skill-upper - - name: Make reports readable - # The Docker container action runs as root, so report files land - # root-owned; relax perms before the runner-user upload step reads them. + - name: Restore report ownership + # The Docker container action runs as root. The action entrypoint fixes + # this on exit; keep a host-side fallback so artifact upload and the + # next checkout can read and remove generated reports on self-hosted + # runners. if: always() && steps.secrets.outputs.available == 'true' - run: sudo chmod -R a+rX "$GITHUB_WORKSPACE/skill-up-workspace" || true + run: | + sudo chown -R "$(id -u):$(id -g)" "$GITHUB_WORKSPACE/skill-up-workspace" || true + chmod -R u+rwX,go+rX "$GITHUB_WORKSPACE/skill-up-workspace" || true - name: Upload eval report if: always() && steps.secrets.outputs.available == 'true' diff --git a/CHANGELOG.md b/CHANGELOG.md index 50d9d77..02d27e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `result.json`; turn-scoped failure details in JUnit XML). ### Changed +- `agent_judge` now defaults to materialized review context (`profile: + standard`) instead of inlining full transcript and workspace diff into the + judge prompt. The judge receives a materials table with file references for + large artifacts, while short final messages remain inline. Evals that require + legacy inline transcript can set `judge.context.transcript: include` subject + to `judge.context.limits.max_bytes`. - `skill-up run` now validates only the cases left after `--include-case-name` / `--exclude-case-name` filters (plus the eval-level config), so an invalid case that is filtered out no longer blocks a filtered @@ -30,6 +36,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 case) regardless of filters. ### Added +- `judge.context` for `agent_judge`, with `minimal` and `standard` profiles, + per-field delivery modes, inline-size limits, generated-file indexing, and + attachment file references. Reports now include `judge_context` metadata with + materialization and prompt-delivery details. +- Built-in CLI agents now route large prompts through prompt delivery, writing + prompt artifacts to disk and feeding runtime-readable prompt files through + stdin when the prompt exceeds `SKILL_UP_PROMPT_INLINE_MAX_BYTES` (default + 32768 bytes). - Config validation now rejects duplicate case IDs and duplicate `cases.files` references. Since reports and artifacts are keyed by case ID and one case runs per `cases.files` entry, a collision would silently overwrite results or diff --git a/action.yml b/action.yml index 0c475da..c035557 100644 --- a/action.yml +++ b/action.yml @@ -65,7 +65,7 @@ outputs: # Docker container action: the prebuilt image ships the three engines + skill-up # + the action code, so the caller just `uses:` it. The image is published to -# this repo's own org namespace by .github/workflows/build-runner-image.yml; +# this repo's own org namespace by .github/workflows/runner-image.yml; # bump the tag together with releases (pin by @sha256 digest for reproducibility). # NOTE: Docker container actions run on Linux runners only. runs: diff --git a/action/Dockerfile b/action/Dockerfile index 8806b78..af39377 100644 --- a/action/Dockerfile +++ b/action/Dockerfile @@ -2,7 +2,7 @@ # # Bakes the agent engines + skill-up + a python3 interpreter + the action # code, so the action itself is a thin Docker container action that just runs. -# Built and pushed to ghcr.io by .github/workflows/build-runner-image.yml. +# Built and pushed to ghcr.io by .github/workflows/runner-image.yml. # # Everything installed here is from PUBLIC registries (npm / GitHub Releases) — # no internal Alibaba network is required, so this builds on github.com hosted diff --git a/action/entry.sh b/action/entry.sh index 9f2b240..714b0a6 100755 --- a/action/entry.sh +++ b/action/entry.sh @@ -17,9 +17,35 @@ export PATH set -e +restore_workspace_permissions() { + local workspace="${GITHUB_WORKSPACE:-}" + [ -n "$workspace" ] || return 0 + + local report_dir="$workspace/skill-up-workspace" + [ -e "$report_dir" ] || return 0 + + # Docker actions run as root and write into the host-mounted workspace. + # Hand generated reports back to the runner user so the next checkout can + # clean the workspace on self-hosted runners. + local owner + owner="$(stat -c '%u:%g' "$workspace" 2>/dev/null || true)" + if [ -n "$owner" ]; then + chown -R "$owner" "$report_dir" 2>/dev/null || true + fi + chmod -R u+rwX,go+rX "$report_dir" 2>/dev/null || true +} + +cleanup() { + local status=$? + restore_workspace_permissions + exit "$status" +} +trap cleanup EXIT + for c in python3 python /usr/bin/python3 /usr/local/bin/python3; do if command -v "$c" >/dev/null 2>&1 || [ -x "$c" ]; then - exec "$c" "$DIR/main.py" "$@" + "$c" "$DIR/main.py" "$@" + exit 0 fi done diff --git a/docs/guide/writing-evals.md b/docs/guide/writing-evals.md index dc54d3e..8a77541 100644 --- a/docs/guide/writing-evals.md +++ b/docs/guide/writing-evals.md @@ -667,6 +667,43 @@ judge: timeout_seconds: 60 # Optional: bound a single judge call (0 = no judge-level deadline, parent case timeout still applies) ``` +`agent_judge` materializes review context into files and injects a small +materials table into the judge prompt. When `judge.context` is omitted, the +default profile is `standard`: `final_message` is included inline, while +`transcript` and `workspace_diff` are provided as file references to avoid +large prompt/argv failures. + +Use `minimal` for long repository-change benchmarks where the judge should rely +on explicit attachments or script outputs instead of the full conversation: + +```yaml +judge: + type: agent_judge + model: anthropic/claude-sonnet-4-6 + context: + profile: minimal # transcript/diff omitted, final_message truncated + attachments: + - path: evals/fixtures/diff-result.json + label: diff_result + criteria: + - "Determine whether the reported changes satisfy the expected rules." +``` + +Per-field modes are available when you need to tune the profile: + +```yaml +judge: + type: agent_judge + context: + profile: standard + final_message: include # include | truncate | file_ref | omit + transcript: file_ref # include auto-downgrades to file_ref above limits.max_bytes + workspace_diff: file_ref + generated_files: index # index | include | omit + limits: + max_bytes: 65536 +``` + > **Cost note:** `agent_judge` consumes additional tokens. Prefer `expect` or `rule_based` for deterministic checks and reserve `agent_judge` for assertions that genuinely require semantic understanding. --- diff --git a/docs/zh/guide/writing-evals.md b/docs/zh/guide/writing-evals.md index 1f83301..5cfc167 100644 --- a/docs/zh/guide/writing-evals.md +++ b/docs/zh/guide/writing-evals.md @@ -642,6 +642,42 @@ judge: timeout_seconds: 60 # 可选:限制单次 judge 调用时长(0 = 不加 judge 级 deadline,仍受 case timeout 约束) ``` +`agent_judge` 会将评审上下文物化为文件,并在 judge prompt 中注入一个很小的 +材料表。未配置 `judge.context` 时,默认使用 `standard` profile: +`final_message` 内联,`transcript` 和 `workspace_diff` 以文件引用形式提供, +避免超长 prompt/argv 导致执行失败。 + +长时间运行的仓库变更类评测通常适合使用 `minimal`,让 judge 依赖显式附件或脚本 +输出,而不是完整对话历史: + +```yaml +judge: + type: agent_judge + model: anthropic/claude-sonnet-4-6 + context: + profile: minimal # 省略 transcript/diff,截断 final_message + attachments: + - path: evals/fixtures/diff-result.json + label: diff_result + criteria: + - "判断报告中的代码变更是否满足预期规则。" +``` + +如需微调 profile,可按字段指定模式: + +```yaml +judge: + type: agent_judge + context: + profile: standard + final_message: include # include | truncate | file_ref | omit + transcript: file_ref # include 超过 limits.max_bytes 会自动降级为 file_ref + workspace_diff: file_ref + generated_files: index # index | include | omit + limits: + max_bytes: 65536 +``` + > **成本提示**:`agent_judge` 会消耗额外的 token。建议对关键断言先用 `expect` 或 `rule_based` 做确定性检查,只对需要语义理解的部分使用 `agent_judge`。 --- diff --git a/e2e/agent_test.go b/e2e/agent_test.go index c4c0a97..8070f4d 100644 --- a/e2e/agent_test.go +++ b/e2e/agent_test.go @@ -138,22 +138,27 @@ if [[ "$instruction" == *"Required Response Format (JSON)"* ]]; then passed=true evidence="workspace diff captured expected change" case_id="unknown" + review_text="$instruction" + diff_path="$(printf '%s\n' "$instruction" | tr '|' '\n' | awk '{for (i = 1; i <= NF; i++) if ($i ~ /workspace\.diff$/) print $i}' | head -n 1)" + if [[ -n "$diff_path" && -f "$diff_path" ]]; then + review_text="${review_text}"$'\n'"$(cat "$diff_path")" + fi if [[ "$instruction" == *"git-init-workspace"* ]]; then case_id="git-init-workspace" - [[ "$instruction" == *"diff --git a/README.md b/README.md"* ]] || passed=false + [[ "$review_text" == *"diff --git a/README.md b/README.md"* ]] || passed=false elif [[ "$instruction" == *"cloned-git-workspace"* ]]; then case_id="cloned-git-workspace" - [[ "$instruction" == *"diff --git a/repo.txt b/repo.txt"* ]] || passed=false + [[ "$review_text" == *"diff --git a/repo.txt b/repo.txt"* ]] || passed=false else passed=false fi write_debug_file "judge-${case_id}.txt" "$instruction" - [[ "$instruction" == *"-before"* ]] || passed=false - [[ "$instruction" == *"+after"* ]] || passed=false - if [[ "$instruction" == *"stdout.json"* ]]; then + [[ "$review_text" == *"-before"* ]] || passed=false + [[ "$review_text" == *"+after"* ]] || passed=false + if [[ "$review_text" == *"stdout.json"* ]]; then passed=false evidence="workspace diff leaked generated artifacts" fi diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 64c223d..b5cf8d0 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -51,18 +51,19 @@ var ErrAgentInstallFailed = errors.New("agent installation failed") // SessionResult holds the result of an agent session execution. type SessionResult struct { - Engine string `json:"engine,omitempty"` - Model string `json:"model,omitempty"` - SessionID string `json:"session_id,omitempty"` - ExitCode int `json:"exit_code"` - DurationMs int64 `json:"duration_ms"` - Turns int `json:"turns"` - InputTokens int `json:"input_tokens,omitempty"` - OutputTokens int `json:"output_tokens,omitempty"` - FinalMessage string `json:"final_message,omitempty"` - Stderr string `json:"stderr,omitempty"` - Transcript transcript.Transcript `json:"transcript,omitempty"` - Artifacts *SessionArtifacts `json:"artifacts,omitempty"` + Engine string `json:"engine,omitempty"` + Model string `json:"model,omitempty"` + SessionID string `json:"session_id,omitempty"` + ExitCode int `json:"exit_code"` + DurationMs int64 `json:"duration_ms"` + Turns int `json:"turns"` + InputTokens int `json:"input_tokens,omitempty"` + OutputTokens int `json:"output_tokens,omitempty"` + FinalMessage string `json:"final_message,omitempty"` + Stderr string `json:"stderr,omitempty"` + Transcript transcript.Transcript `json:"transcript,omitempty"` + Artifacts *SessionArtifacts `json:"artifacts,omitempty"` + PromptDelivery *PromptDeliveryMetadata `json:"prompt_delivery,omitempty"` } // SessionResumer is an optional interface that agents may implement to support diff --git a/internal/agent/claude_code.go b/internal/agent/claude_code.go index d614d9b..ef3082c 100644 --- a/internal/agent/claude_code.go +++ b/internal/agent/claude_code.go @@ -147,24 +147,31 @@ func (a *ClaudeCodeAgent) Run(ctx context.Context, rt Runtime, opts ExecOptions, Artifacts: &SessionArtifacts{}, }, err } - cmd := buildClaudePrintCmd(sessionID, a.effectiveModelName(ctx), instruction) + cmd, promptDelivery, err := deliverPrompt(ctx, rt, opts, instruction, promptCommandBuilder{ + Inline: func(prompt string) string { + return buildClaudePrintCmd(sessionID, a.effectiveModelName(ctx), prompt) + }, + StdinFile: func(path string) string { + return buildClaudePrintStdinCmd(sessionID, a.effectiveModelName(ctx), path) + }, + }) + if err != nil { + return &SessionResult{ + Engine: a.Name(), + ExitCode: 1, + DurationMs: time.Since(start).Milliseconds(), + Artifacts: &SessionArtifacts{}, + }, err + } result, err := rt.Exec(ctx, cmd, opts) sessionResult := a.buildSessionResult(ctx, rt, opts, instruction, start, result) if sessionResult != nil { + sessionResult.PromptDelivery = promptDelivery sessionResult.SessionID = sessionID } - if authMsg, ok := providerAuthFailureSignal(result, sessionResult); ok { - if sessionResult != nil && sessionResult.ExitCode == 0 { - sessionResult.ExitCode = 1 - } - return sessionResult, fmt.Errorf("claude-code authentication failed: %s", authMsg) - } - if rateLimitMsg, ok := providerRateLimitSignal(result, sessionResult); ok { - if sessionResult != nil && sessionResult.ExitCode == 0 { - sessionResult.ExitCode = 1 - } - return sessionResult, fmt.Errorf("claude-code provider rate limit: %s", rateLimitMsg) + if providerErr := claudeProviderRunError(result, sessionResult); providerErr != nil { + return sessionResult, providerErr } if err != nil { if sessionResult == nil { @@ -190,6 +197,24 @@ func (a *ClaudeCodeAgent) effectiveModelName(_ context.Context) string { return strings.TrimSpace(a.Cfg.ModelName) } +func claudeProviderRunError(result ExecResult, sessionResult *SessionResult) error { + if authMsg, ok := providerAuthFailureSignal(result, sessionResult); ok { + markSessionFailed(sessionResult) + return fmt.Errorf("claude-code authentication failed: %s", authMsg) + } + if rateLimitMsg, ok := providerRateLimitSignal(result, sessionResult); ok { + markSessionFailed(sessionResult) + return fmt.Errorf("claude-code provider rate limit: %s", rateLimitMsg) + } + return nil +} + +func markSessionFailed(sessionResult *SessionResult) { + if sessionResult != nil && sessionResult.ExitCode == 0 { + sessionResult.ExitCode = 1 + } +} + func providerRateLimitSignal(result ExecResult, sessionResult *SessionResult) (string, bool) { for _, candidate := range []string{ result.Stderr, @@ -292,6 +317,14 @@ func buildClaudePrintCmd(sessionID, model, instruction string) string { return cmd } +func buildClaudePrintStdinCmd(sessionID, model, promptPath string) string { + cmd := "cat " + shellQuote(promptPath) + " | claude --settings " + shellQuote(`{"disableAllHooks":true}`) + " --session-id " + sessionID + " -p --permission-mode=bypassPermissions" + if model != "" { + cmd += " --model " + shellQuote(model) + } + return cmd +} + // buildClaudeResumeCmd constructs a claude CLI command that resumes an existing // session identified by sessionID using --resume and sends a new prompt. func buildClaudeResumeCmd(sessionID, model, instruction string) string { diff --git a/internal/agent/codex.go b/internal/agent/codex.go index 0e7d295..7f76498 100644 --- a/internal/agent/codex.go +++ b/internal/agent/codex.go @@ -325,11 +325,29 @@ func (a *CodexAgent) Run(ctx context.Context, rt Runtime, opts ExecOptions, mess Artifacts: &SessionArtifacts{}, }, err } - cmd := "mkdir -p " + shellQuote(filepath.Dir(lastMessagePath)) + "\n" + - buildCodexRunCmdWithLastMessage(instruction, a.effectiveModelName(ctx), a.runProviderConfig(ctx), sandboxFlag, lastMessagePath) + runCmd, promptDelivery, err := deliverPrompt(ctx, rt, opts, instruction, promptCommandBuilder{ + Inline: func(prompt string) string { + return buildCodexRunCmdWithLastMessage(prompt, a.effectiveModelName(ctx), a.runProviderConfig(ctx), sandboxFlag, lastMessagePath) + }, + StdinFile: func(path string) string { + return buildCodexRunStdinCmdWithLastMessage(path, a.effectiveModelName(ctx), a.runProviderConfig(ctx), sandboxFlag, lastMessagePath) + }, + }) + if err != nil { + return &SessionResult{ + Engine: a.Name(), + ExitCode: 1, + DurationMs: time.Since(start).Milliseconds(), + Artifacts: &SessionArtifacts{}, + }, err + } + cmd := "mkdir -p " + shellQuote(filepath.Dir(lastMessagePath)) + "\n" + runCmd result, err := rt.Exec(ctx, cmd, opts) sessionResult := a.buildSessionResult(ctx, rt, opts, instruction, start, result, lastMessagePath) + if sessionResult != nil { + sessionResult.PromptDelivery = promptDelivery + } if err != nil { if sessionResult == nil { sessionResult = &SessionResult{ @@ -532,6 +550,21 @@ func buildCodexRunCmdWithLastMessage(instruction, model string, provider codexPr return cmd } +func buildCodexRunStdinCmdWithLastMessage(promptPath, model string, provider codexProviderConfig, sandboxFlag, lastMessagePath string) string { + cmd := "cat " + shellQuote(promptPath) + " | codex exec --json --skip-git-repo-check" + if sandboxFlag != "" { + cmd += " " + sandboxFlag + } + cmd += codexProviderFlags(provider) + if model != "" { + cmd += " -m " + shellQuote(model) + } + if lastMessagePath != "" { + cmd += " --output-last-message " + shellQuote(lastMessagePath) + } + return cmd +} + // buildCodexResumeCmdWithLastMessage constructs a codex CLI command that resumes // an existing session identified by sessionID and sends a new user message. // Note: `codex exec resume` does not support --sandbox; the session inherits diff --git a/internal/agent/prompt_delivery.go b/internal/agent/prompt_delivery.go new file mode 100644 index 0000000..dd35544 --- /dev/null +++ b/internal/agent/prompt_delivery.go @@ -0,0 +1,93 @@ +package agent + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + + "github.com/alibaba/skill-up/internal/logging" +) + +const ( + defaultPromptInlineMaxBytes = 32 * 1024 + envPromptInlineMaxBytes = "SKILL_UP_PROMPT_INLINE_MAX_BYTES" +) + +// PromptDeliveryMetadata records how an agent prompt was delivered. +type PromptDeliveryMetadata struct { + Mode string `json:"mode"` + PromptBytes int `json:"prompt_bytes"` + InlineMaxBytes int `json:"inline_max_bytes"` + PromptPath string `json:"prompt_path,omitempty"` + RuntimePath string `json:"runtime_path,omitempty"` +} + +type promptCommandBuilder struct { + Inline func(string) string + StdinFile func(string) string +} + +func deliverPrompt(ctx context.Context, rt Runtime, opts ExecOptions, instruction string, builder promptCommandBuilder) (string, *PromptDeliveryMetadata, error) { + threshold := promptInlineMaxBytes() + promptBytes := len([]byte(instruction)) + meta := &PromptDeliveryMetadata{ + Mode: "inline", + PromptBytes: promptBytes, + InlineMaxBytes: threshold, + } + if builder.Inline == nil { + return "", meta, errors.New("prompt delivery requires inline command builder") + } + if promptBytes <= threshold { + return builder.Inline(instruction), meta, nil + } + if builder.StdinFile == nil { + return "", meta, fmt.Errorf("prompt delivery requires stdin file command builder for %d-byte prompt above %d-byte inline threshold", promptBytes, threshold) + } + if rt == nil { + return "", meta, errors.New("prompt delivery requires runtime for file mode") + } + + runtimePath := filepath.Join(rt.Workspace(), ".skill-up", "prompts", "prompt.txt") + if err := persistRuntimeArtifact(ctx, rt, runtimePath, instruction); err != nil { + return "", nil, fmt.Errorf("deliver prompt file: %w", err) + } + hostPath, err := mirrorPromptArtifact(opts.ArtifactDir, instruction) + if err != nil { + return "", nil, err + } + meta.Mode = "file" + meta.PromptPath = hostPath + meta.RuntimePath = runtimePath + logging.InfoContextf(ctx, "prompt delivery mode=file bytes=%d threshold=%d", meta.PromptBytes, threshold) + return builder.StdinFile(runtimePath), meta, nil +} + +func promptInlineMaxBytes() int { + raw := os.Getenv(envPromptInlineMaxBytes) + if raw == "" { + return defaultPromptInlineMaxBytes + } + n, err := strconv.Atoi(raw) + if err != nil || n < 0 { + return defaultPromptInlineMaxBytes + } + return n +} + +func mirrorPromptArtifact(artifactDir, instruction string) (string, error) { + if artifactDir == "" { + return "", nil + } + if err := os.MkdirAll(artifactDir, 0o755); err != nil { + return "", fmt.Errorf("create prompt artifact dir: %w", err) + } + path := filepath.Join(artifactDir, "prompt.txt") + if err := os.WriteFile(path, []byte(instruction), 0o600); err != nil { + return "", fmt.Errorf("write prompt artifact: %w", err) + } + return path, nil +} diff --git a/internal/agent/prompt_delivery_test.go b/internal/agent/prompt_delivery_test.go new file mode 100644 index 0000000..346ac52 --- /dev/null +++ b/internal/agent/prompt_delivery_test.go @@ -0,0 +1,145 @@ +package agent + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/alibaba/skill-up/internal/runtime" +) + +func TestDeliverPrompt_InlineBelowThreshold(t *testing.T) { + t.Setenv(envPromptInlineMaxBytes, "16") + rt := &promptDeliveryTestRuntime{workspace: t.TempDir()} + + cmd, meta, err := deliverPrompt(context.Background(), rt, ExecOptions{ArtifactDir: t.TempDir()}, "short", promptCommandBuilder{ + Inline: func(prompt string) string { return "run " + shellQuote(prompt) }, + StdinFile: func(path string) string { + return "cat " + shellQuote(path) + " | run" + }, + }) + if err != nil { + t.Fatalf("deliverPrompt returned error: %v", err) + } + if cmd != "run 'short'" { + t.Fatalf("unexpected inline command: %q", cmd) + } + if meta.Mode != "inline" { + t.Fatalf("mode = %q, want inline", meta.Mode) + } + if rt.uploadedTarget != "" { + t.Fatalf("did not expect upload for inline prompt, got %q", rt.uploadedTarget) + } +} + +func TestDeliverPrompt_FileAboveThreshold(t *testing.T) { + t.Setenv(envPromptInlineMaxBytes, "16") + artifactDir := t.TempDir() + workspace := t.TempDir() + rt := &promptDeliveryTestRuntime{workspace: workspace} + instruction := strings.Repeat("x", 64) + + cmd, meta, err := deliverPrompt(context.Background(), rt, ExecOptions{ArtifactDir: artifactDir}, instruction, promptCommandBuilder{ + Inline: func(prompt string) string { return "run " + shellQuote(prompt) }, + StdinFile: func(path string) string { + return "cat " + shellQuote(path) + " | run" + }, + }) + if err != nil { + t.Fatalf("deliverPrompt returned error: %v", err) + } + if meta.Mode != "file" { + t.Fatalf("mode = %q, want file", meta.Mode) + } + if strings.Contains(cmd, instruction) { + t.Fatalf("file-mode command should not contain full instruction: %q", cmd) + } + if !strings.Contains(cmd, ".skill-up/prompts/prompt.txt") { + t.Fatalf("file-mode command should reference runtime prompt path: %q", cmd) + } + if meta.PromptPath != filepath.Join(artifactDir, "prompt.txt") { + t.Fatalf("PromptPath = %q", meta.PromptPath) + } + data, err := os.ReadFile(meta.PromptPath) + if err != nil { + t.Fatalf("read prompt artifact: %v", err) + } + if string(data) != instruction { + t.Fatalf("prompt artifact mismatch") + } + if rt.uploadedContent != instruction { + t.Fatalf("uploaded content mismatch") + } +} + +func TestDeliverPrompt_RequiresInlineBuilder(t *testing.T) { + t.Setenv(envPromptInlineMaxBytes, "16") + rt := &promptDeliveryTestRuntime{workspace: t.TempDir()} + + _, _, err := deliverPrompt(context.Background(), rt, ExecOptions{}, "short", promptCommandBuilder{}) + if err == nil { + t.Fatal("expected missing inline builder error") + } + if !strings.Contains(err.Error(), "inline command builder") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestDeliverPrompt_RejectsOversizedPromptWithoutStdinFileBuilder(t *testing.T) { + t.Setenv(envPromptInlineMaxBytes, "16") + rt := &promptDeliveryTestRuntime{workspace: t.TempDir()} + instruction := strings.Repeat("x", 64) + + cmd, meta, err := deliverPrompt(context.Background(), rt, ExecOptions{}, instruction, promptCommandBuilder{ + Inline: func(prompt string) string { return "run " + shellQuote(prompt) }, + }) + if err == nil { + t.Fatal("expected oversized prompt error") + } + if cmd != "" { + t.Fatalf("unexpected command on error: %q", cmd) + } + if meta == nil || meta.Mode != "inline" { + t.Fatalf("expected inline metadata on error, got %#v", meta) + } + if !strings.Contains(err.Error(), "stdin file command builder") { + t.Fatalf("unexpected error: %v", err) + } + if rt.uploadedTarget != "" { + t.Fatalf("did not expect upload on error, got %q", rt.uploadedTarget) + } +} + +type promptDeliveryTestRuntime struct { + workspace string + uploadedTarget string + uploadedContent string +} + +func (r *promptDeliveryTestRuntime) Create(context.Context) error { return nil } +func (r *promptDeliveryTestRuntime) Close() error { return nil } +func (r *promptDeliveryTestRuntime) Start(context.Context) error { return nil } +func (r *promptDeliveryTestRuntime) Stop(context.Context) error { return nil } +func (r *promptDeliveryTestRuntime) UploadFile(_ context.Context, sourcePath, targetPath string) error { + data, err := os.ReadFile(sourcePath) + if err != nil { + return err + } + r.uploadedTarget = targetPath + r.uploadedContent = string(data) + return nil +} +func (r *promptDeliveryTestRuntime) UploadDir(context.Context, string, string) error { return nil } +func (r *promptDeliveryTestRuntime) DownloadFile(context.Context, string, string) error { + return nil +} +func (r *promptDeliveryTestRuntime) DownloadDir(context.Context, string, string) error { return nil } +func (r *promptDeliveryTestRuntime) Exec(context.Context, string, runtime.ExecOptions) (runtime.ExecResult, error) { + return runtime.ExecResult{}, nil +} +func (r *promptDeliveryTestRuntime) MergeEnv(map[string]string) {} +func (r *promptDeliveryTestRuntime) Workspace() string { return r.workspace } +func (r *promptDeliveryTestRuntime) RequiresProcessSandbox() bool { return false } +func (r *promptDeliveryTestRuntime) TargetGOOS() string { return "linux" } diff --git a/internal/agent/qodercli.go b/internal/agent/qodercli.go index 4081d44..2524333 100644 --- a/internal/agent/qodercli.go +++ b/internal/agent/qodercli.go @@ -76,7 +76,22 @@ func (a *QoderCLIAgent) Run(ctx context.Context, rt Runtime, opts ExecOptions, m start := time.Now() instruction := BuildInstructionFromMessages(messages) - cmd := buildQoderRunCmd(instruction, a.effectiveModelName(ctx)) + cmd, promptDelivery, err := deliverPrompt(ctx, rt, opts, instruction, promptCommandBuilder{ + Inline: func(prompt string) string { + return buildQoderRunCmd(prompt, a.effectiveModelName(ctx)) + }, + StdinFile: func(path string) string { + return buildQoderRunStdinCmd(path, a.effectiveModelName(ctx)) + }, + }) + if err != nil { + return &SessionResult{ + Engine: a.Name(), + ExitCode: 1, + DurationMs: time.Since(start).Milliseconds(), + Artifacts: &SessionArtifacts{}, + }, err + } envVars := a.credentialEnvVars("", "") opts = a.mergeExecOptionsEnv(ctx, opts, envVars, a.buildAgentObservabilityAttrs(nil)) @@ -84,6 +99,9 @@ func (a *QoderCLIAgent) Run(ctx context.Context, rt Runtime, opts ExecOptions, m result, err := rt.Exec(ctx, cmd, opts) sessionResult := a.buildSessionResult(ctx, rt, opts, instruction, start, result) + if sessionResult != nil { + sessionResult.PromptDelivery = promptDelivery + } if err != nil { if sessionResult == nil { sessionResult = &SessionResult{ @@ -129,6 +147,16 @@ func buildQoderRunCmd(instruction, model string) string { return cmd } +func buildQoderRunStdinCmd(promptPath, model string) string { + cmd := "cat " + shellQuote(promptPath) + " | qodercli --permission-mode=bypass_permissions" + if model != "" { + cmd += " --model " + shellQuote(model) + } + cmd += " -p -" + + return cmd +} + // buildQoderResumeCmd constructs a qodercli command that resumes an existing // session identified by sessionID and sends a new user prompt. func buildQoderResumeCmd(instruction, model, sessionID string) string { diff --git a/internal/agent/qwen_code.go b/internal/agent/qwen_code.go index 509dd3c..991b39a 100644 --- a/internal/agent/qwen_code.go +++ b/internal/agent/qwen_code.go @@ -164,9 +164,27 @@ func (a *QwenCodeAgent) Run(ctx context.Context, rt Runtime, opts ExecOptions, m }, err } - cmd := buildQwenCodeRunCmd(instruction, model) + cmd, promptDelivery, err := deliverPrompt(ctx, rt, opts, instruction, promptCommandBuilder{ + Inline: func(prompt string) string { + return buildQwenCodeRunCmd(prompt, model) + }, + StdinFile: func(path string) string { + return buildQwenCodeRunStdinCmd(path, model) + }, + }) + if err != nil { + return &SessionResult{ + Engine: a.Name(), + ExitCode: 1, + DurationMs: time.Since(start).Milliseconds(), + Artifacts: &SessionArtifacts{}, + }, err + } result, err := rt.Exec(ctx, cmd, opts) sessionResult := a.buildSessionResult(ctx, rt, opts, instruction, start, result) + if sessionResult != nil { + sessionResult.PromptDelivery = promptDelivery + } if err != nil { return sessionResult, fmt.Errorf("qwen_code run failed: %w", err) } @@ -196,6 +214,14 @@ func buildQwenCodeRunCmd(instruction, model string) string { return cmd } +func buildQwenCodeRunStdinCmd(promptPath, model string) string { + cmd := "cat " + shellQuote(promptPath) + " | qwen --yolo" + if model != "" { + cmd += " -m " + shellQuote(model) + } + return cmd +} + // buildSessionResult prefers qwen's own session JSONL (full transcript, tool // calls and usageMetadata token counts) when it can be located and downloaded, // and falls back to a minimal stdout-derived transcript otherwise. diff --git a/internal/config/schema.go b/internal/config/schema.go index cc12e0e..7a12235 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -186,10 +186,11 @@ type RetryPolicy struct { // JudgeConfig describes the evaluation strategy. type JudgeConfig struct { - Type string `json:"type" yaml:"type"` // rule_based, script, agent_judge - ScriptPath string `json:"script_path,omitempty" yaml:"script_path,omitempty"` - Model string `json:"model,omitempty" yaml:"model,omitempty"` - Criteria []string `json:"criteria,omitempty" yaml:"criteria,omitempty"` + Type string `json:"type" yaml:"type"` // rule_based, script, agent_judge + ScriptPath string `json:"script_path,omitempty" yaml:"script_path,omitempty"` + Model string `json:"model,omitempty" yaml:"model,omitempty"` + Criteria []string `json:"criteria,omitempty" yaml:"criteria,omitempty"` + Context *JudgeContextConfig `json:"context,omitempty" yaml:"context,omitempty"` // PassThreshold is the minimum pass rate for agent_judge. // Nil means "not configured", so the judge layer applies its default of 0.7. // When set explicitly, the value must be in the inclusive range [0.0, 1.0]. @@ -204,6 +205,30 @@ type JudgeConfig struct { Failure []Rule `json:"failure,omitempty" yaml:"failure,omitempty"` } +// JudgeContextConfig controls how agent_judge receives evaluation materials. +type JudgeContextConfig struct { + Profile string `json:"profile,omitempty" yaml:"profile,omitempty"` + FinalMessage string `json:"final_message,omitempty" yaml:"final_message,omitempty"` + Transcript string `json:"transcript,omitempty" yaml:"transcript,omitempty"` + WorkspaceDiff string `json:"workspace_diff,omitempty" yaml:"workspace_diff,omitempty"` + GeneratedFiles string `json:"generated_files,omitempty" yaml:"generated_files,omitempty"` + Limits *JudgeContextLimits `json:"limits,omitempty" yaml:"limits,omitempty"` + Attachments []JudgeContextAttachment `json:"attachments,omitempty" yaml:"attachments,omitempty"` +} + +// JudgeContextLimits bounds inline material size for agent_judge prompts. +type JudgeContextLimits struct { + MaxBytes int `json:"max_bytes,omitempty" yaml:"max_bytes,omitempty"` + TranscriptMaxTurns int `json:"transcript_max_turns,omitempty" yaml:"transcript_max_turns,omitempty"` + WorkspaceDiffMaxLines int `json:"workspace_diff_max_lines,omitempty" yaml:"workspace_diff_max_lines,omitempty"` +} + +// JudgeContextAttachment declares an additional file for agent_judge review. +type JudgeContextAttachment struct { + Path string `json:"path" yaml:"path"` + Label string `json:"label,omitempty" yaml:"label,omitempty"` +} + // Rule is a single assertion rule for rule_based evaluation. type Rule struct { OutputContains *OutputContainsRule `json:"output_contains,omitempty" yaml:"output_contains,omitempty"` diff --git a/internal/config/validator.go b/internal/config/validator.go index b05b303..c4e3197 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -96,6 +96,7 @@ func (v *Validator) ValidateEvalConfig(cfg *EvalConfig) error { } errs = append(errs, validateCollectArtifacts("cases.defaults.collect_artifacts", cfg.Cases.Defaults.CollectArtifacts)...) + errs = append(errs, validateJudgeContext(cfg.Judge.Context)...) if len(errs) > 0 { return fmt.Errorf("validation errors:\n - %s", strings.Join(errs, "\n - ")) @@ -182,6 +183,7 @@ func validateJudgeTypeAndFields(judge JudgeConfig) []string { } errs = append(errs, validatePassThreshold(judge.PassThreshold)...) + errs = append(errs, validateJudgeContext(judge.Context)...) if judge.TimeoutSeconds != nil && *judge.TimeoutSeconds < 0 { errs = append(errs, "judge.timeout_seconds must be non-negative") @@ -217,6 +219,68 @@ func validatePassThreshold(threshold *float64) []string { return nil } +func validateJudgeContext(ctx *JudgeContextConfig) []string { + if ctx == nil { + return nil + } + + var errs []string + if ctx.Profile != "" && ctx.Profile != "minimal" && ctx.Profile != "standard" { + errs = append(errs, "judge.context.profile must be one of: minimal, standard") + } + errs = append(errs, validateJudgeContextModes(ctx)...) + errs = append(errs, validateJudgeContextLimits(ctx.Limits)...) + for i, attachment := range ctx.Attachments { + if strings.TrimSpace(attachment.Path) == "" { + errs = append(errs, fmt.Sprintf("judge.context.attachments[%d].path must not be empty", i)) + } + } + return errs +} + +func validateJudgeContextModes(ctx *JudgeContextConfig) []string { + var errs []string + for field, mode := range map[string]string{ + "final_message": ctx.FinalMessage, + "transcript": ctx.Transcript, + "workspace_diff": ctx.WorkspaceDiff, + } { + if mode != "" && !isValidJudgeContextMode(mode) { + errs = append(errs, fmt.Sprintf("judge.context.%s must be one of: include, omit, truncate, file_ref", field)) + } + } + if ctx.GeneratedFiles != "" && ctx.GeneratedFiles != "omit" && ctx.GeneratedFiles != "index" && ctx.GeneratedFiles != "include" { + errs = append(errs, "judge.context.generated_files must be one of: omit, index, include") + } + return errs +} + +func validateJudgeContextLimits(limits *JudgeContextLimits) []string { + if limits == nil { + return nil + } + var errs []string + if limits.MaxBytes < 0 { + errs = append(errs, "judge.context.limits.max_bytes must be non-negative") + } + if limits.TranscriptMaxTurns < 0 { + errs = append(errs, "judge.context.limits.transcript_max_turns must be non-negative") + } + if limits.WorkspaceDiffMaxLines < 0 { + errs = append(errs, "judge.context.limits.workspace_diff_max_lines must be non-negative") + } + return errs +} + +func isValidJudgeContextMode(mode string) bool { + switch mode { + case "include", "omit", "truncate", "file_ref": + return true + default: + return false + } +} + // ValidateAll validates an eval config and all its cases. // // NOTE: this does NOT validate the engine.custom block. That validation is diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 22cd0b6..0a73d24 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -355,6 +355,54 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { }, wantErr: false, }, + { + name: "valid judge context at eval level", + cfg: &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{ + Name: "claude_code", + Model: ModelConfig{ + Provider: "anthropic", + Name: "claude-sonnet-4-6", + }, + }, + Cases: CasesConfig{ + Files: []string{"evals/cases/test.yaml"}, + }, + Judge: JudgeConfig{ + Context: &JudgeContextConfig{ + Profile: "standard", + Transcript: "file_ref", + GeneratedFiles: "index", + Attachments: []JudgeContextAttachment{{Path: "fixtures/result.json"}}, + }, + }, + }, + wantErr: false, + }, + { + name: "invalid judge context at eval level", + cfg: &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{ + Name: "claude_code", + Model: ModelConfig{ + Provider: "anthropic", + Name: "claude-sonnet-4-6", + }, + }, + Cases: CasesConfig{ + Files: []string{"evals/cases/test.yaml"}, + }, + Judge: JudgeConfig{ + Context: &JudgeContextConfig{Profile: "legacy"}, + }, + }, + wantErr: true, + errMsg: "judge.context.profile must be one of: minimal, standard", + }, { name: "script without script_path at eval level is ignored", cfg: &EvalConfig{ @@ -859,6 +907,81 @@ func TestValidator_ValidateCaseConfig(t *testing.T) { wantErr: true, errMsg: "judge.timeout_seconds must be non-negative", }, + { + name: "case-level agent_judge with valid context", + cfg: &CaseConfig{ + ID: "test-case", + Input: Input{ + Prompt: "Say hello", + }, + Judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Context: &JudgeContextConfig{ + Profile: "minimal", + FinalMessage: "truncate", + Transcript: "omit", + WorkspaceDiff: "file_ref", + GeneratedFiles: "index", + Limits: &JudgeContextLimits{MaxBytes: 1024}, + Attachments: []JudgeContextAttachment{{Path: "fixtures/result.json"}}, + }, + }, + }, + wantErr: false, + }, + { + name: "case-level agent_judge with invalid context profile", + cfg: &CaseConfig{ + ID: "test-case", + Input: Input{ + Prompt: "Say hello", + }, + Judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Context: &JudgeContextConfig{Profile: "legacy"}, + }, + }, + wantErr: true, + errMsg: "judge.context.profile must be one of: minimal, standard", + }, + { + name: "case-level agent_judge with invalid context mode", + cfg: &CaseConfig{ + ID: "test-case", + Input: Input{ + Prompt: "Say hello", + }, + Judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Context: &JudgeContextConfig{Transcript: "inline"}, + }, + }, + wantErr: true, + errMsg: "judge.context.transcript must be one of: include, omit, truncate, file_ref", + }, + { + name: "case-level agent_judge with empty attachment path", + cfg: &CaseConfig{ + ID: "test-case", + Input: Input{ + Prompt: "Say hello", + }, + Judge: JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"criterion"}, + Context: &JudgeContextConfig{Attachments: []JudgeContextAttachment{{}}}, + }, + }, + wantErr: true, + errMsg: "judge.context.attachments[0].path must not be empty", + }, } for _, tt := range tests { diff --git a/internal/evaluator/evaluator_test.go b/internal/evaluator/evaluator_test.go index e05ea56..0f3c777 100644 --- a/internal/evaluator/evaluator_test.go +++ b/internal/evaluator/evaluator_test.go @@ -1627,14 +1627,10 @@ func TestExecuteCase_AgentJudgeReceivesTranscriptAndWorkspaceDiff(t *testing.T) t.Fatalf("expected PASS status, got %s", result.Status) } if !strings.Contains(judgePrompt, "updated notes.txt") { - t.Fatalf("judge prompt missing transcript content: %s", judgePrompt) - } - if !strings.Contains(judgePrompt, "diff --git a/notes.txt b/notes.txt") { - t.Fatalf("judge prompt missing workspace diff header: %s", judgePrompt) - } - if !strings.Contains(judgePrompt, "-before") || !strings.Contains(judgePrompt, "+after") { - t.Fatalf("judge prompt missing workspace diff body: %s", judgePrompt) + t.Fatalf("judge prompt missing final message inline material: %s", judgePrompt) } + assertJudgePromptReferencesMaterial(t, judgePrompt, "workspace_diff", "workspace.diff") + assertJudgePromptReferencesMaterial(t, judgePrompt, "transcript", "transcript.json") if strings.Contains(judgePrompt, "stdout.json") || strings.Contains(judgePrompt, `"artifact":true`) { t.Fatalf("judge prompt should filter generated artifact diff: %s", judgePrompt) } @@ -1691,6 +1687,13 @@ func captureStdout(t *testing.T, fn func()) string { return buf.String() } +func assertJudgePromptReferencesMaterial(t *testing.T, prompt, key, pathSuffix string) { + t.Helper() + if !strings.Contains(prompt, key) || !strings.Contains(prompt, pathSuffix) { + t.Fatalf("judge prompt missing material reference %s/%s: %s", key, pathSuffix, prompt) + } +} + func TestExecuteCase_AgentJudgeWithoutGitContextSkipsWorkspaceDiff(t *testing.T) { workspace := t.TempDir() if err := os.WriteFile(filepath.Join(workspace, "notes.txt"), []byte("before\n"), 0o600); err != nil { @@ -1773,9 +1776,7 @@ func TestExecuteCase_AgentJudgeWithExistingGitRepoReceivesWorkspaceDiff(t *testi if result.Status != judge.StatusPass { t.Fatalf("expected PASS status, got %s", result.Status) } - if !strings.Contains(*judgePrompt, "diff --git a/notes.txt b/notes.txt") { - t.Fatalf("judge prompt missing workspace diff header: %s", *judgePrompt) - } + assertJudgePromptReferencesMaterial(t, *judgePrompt, "workspace_diff", "workspace.diff") } func TestExecuteCase_AgentJudgeWithClonedGitRepoReceivesWorkspaceDiff(t *testing.T) { @@ -1806,9 +1807,7 @@ func TestExecuteCase_AgentJudgeWithClonedGitRepoReceivesWorkspaceDiff(t *testing if result.Status != judge.StatusPass { t.Fatalf("expected PASS status, got %s", result.Status) } - if !strings.Contains(*judgePrompt, "diff --git a/notes.txt b/notes.txt") { - t.Fatalf("judge prompt missing workspace diff header: %s", *judgePrompt) - } + assertJudgePromptReferencesMaterial(t, *judgePrompt, "workspace_diff", "workspace.diff") } func TestExecuteCase_AgentJudgeWithClonedGitRepoWithoutGlobalConfigReceivesWorkspaceDiff(t *testing.T) { @@ -1846,9 +1845,7 @@ func TestExecuteCase_AgentJudgeWithClonedGitRepoWithoutGlobalConfigReceivesWorks if result.Status != judge.StatusPass { t.Fatalf("expected PASS status, got %s", result.Status) } - if !strings.Contains(*judgePrompt, "diff --git a/notes.txt b/notes.txt") { - t.Fatalf("judge prompt missing workspace diff header: %s", *judgePrompt) - } + assertJudgePromptReferencesMaterial(t, *judgePrompt, "workspace_diff", "workspace.diff") } func TestExecuteCase_AgentJudgeTracksWorkspaceDiffAfterAgentCommit(t *testing.T) { @@ -1904,12 +1901,7 @@ func TestExecuteCase_AgentJudgeTracksWorkspaceDiffAfterAgentCommit(t *testing.T) if result.Status != judge.StatusPass { t.Fatalf("expected PASS status, got %s", result.Status) } - if !strings.Contains(judgePrompt, "diff --git a/notes.txt b/notes.txt") { - t.Fatalf("judge prompt missing workspace diff header: %s", judgePrompt) - } - if !strings.Contains(judgePrompt, "-before") || !strings.Contains(judgePrompt, "+after") { - t.Fatalf("judge prompt missing committed workspace diff body: %s", judgePrompt) - } + assertJudgePromptReferencesMaterial(t, judgePrompt, "workspace_diff", "workspace.diff") } func TestExecuteCase_AgentJudgeWithGitWorktreeReceivesWorkspaceDiff(t *testing.T) { @@ -1943,9 +1935,7 @@ func TestExecuteCase_AgentJudgeWithGitWorktreeReceivesWorkspaceDiff(t *testing.T if result.Status != judge.StatusPass { t.Fatalf("expected PASS status, got %s", result.Status) } - if !strings.Contains(*judgePrompt, "diff --git a/notes.txt b/notes.txt") { - t.Fatalf("judge prompt missing workspace diff header: %s", *judgePrompt) - } + assertJudgePromptReferencesMaterial(t, *judgePrompt, "workspace_diff", "workspace.diff") } func TestExecuteCase_NonAgentJudgeSkipsWorkspaceSnapshot(t *testing.T) { diff --git a/internal/judge/agent_judge.go b/internal/judge/agent_judge.go index 7f7cf0f..e5279a5 100644 --- a/internal/judge/agent_judge.go +++ b/internal/judge/agent_judge.go @@ -9,6 +9,7 @@ import ( "time" "github.com/alibaba/skill-up/internal/agent" + "github.com/alibaba/skill-up/internal/config" "github.com/alibaba/skill-up/internal/logging" "github.com/alibaba/skill-up/internal/runtime" "github.com/alibaba/skill-up/pkg/transcript" @@ -70,10 +71,19 @@ type AgentJudge struct { // TimeoutSeconds bounds a single Evaluate call. <=0 means no judge-level // deadline; the parent context still applies. TimeoutSeconds int + + // Context controls which evaluation materials are written, referenced, or + // inlined for this agent_judge invocation. + Context *config.JudgeContextConfig } // NewAgentJudge creates an AgentJudge with sensible defaults. func NewAgentJudge(ag agent.Agent, rt runtime.Runtime, model string, criteria []string, passThreshold *float64, timeoutSeconds int) *AgentJudge { + return NewAgentJudgeWithContext(ag, rt, model, criteria, passThreshold, nil, timeoutSeconds) +} + +// NewAgentJudgeWithContext creates an AgentJudge with explicit context delivery. +func NewAgentJudgeWithContext(ag agent.Agent, rt runtime.Runtime, model string, criteria []string, passThreshold *float64, contextCfg *config.JudgeContextConfig, timeoutSeconds int) *AgentJudge { threshold := DefaultPassThreshold if passThreshold != nil { threshold = *passThreshold @@ -85,6 +95,7 @@ func NewAgentJudge(ag agent.Agent, rt runtime.Runtime, model string, criteria [] Criteria: criteria, PassThreshold: threshold, TimeoutSeconds: timeoutSeconds, + Context: contextCfg, } } @@ -101,8 +112,13 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { defer cancel() } + materialized, err := MaterializeJudgeContext(ctx, j.Runtime, j.Context, in, in.ArtifactDir) + if err != nil { + return nil, fmt.Errorf("agent_judge materialize context: %w", err) + } + // Build the judge prompt. - prompt := buildJudgePrompt(ctx, j.Criteria, in.FinalMessage, in.WorkspaceDiff, in.Transcript) + prompt := buildJudgePrompt(ctx, j.Criteria, materialized) messages := []transcript.Message{{Role: transcript.RoleUser, Content: prompt, Turn: 1}} // Get criterion results via agent.Agent. Snapshot parentCtx.Err() the @@ -136,26 +152,17 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { } criterionResults := resp.Results - // Validate that the agent evaluated every criterion. - // A short response inflates pass_rate because NewResult uses the returned - // count as the denominator (e.g. 1-of-1 returned = 100% even if 3 were sent). - if len(criterionResults) != len(j.Criteria) { + if err := validateAgentJudgeResponse(j.Criteria, criterionResults); err != nil { return nil, &SessionResultError{ - Err: fmt.Errorf("agent_judge: expected %d criterion results, got %d", len(j.Criteria), len(criterionResults)), + Err: err, Session: sessionResult, } } - // Validate evidence is non-empty for every result (required by prompt). - for i, cr := range criterionResults { - if strings.TrimSpace(cr.Evidence) == "" { - return nil, &SessionResultError{ - Err: fmt.Errorf("agent_judge: criterion %d %q has empty evidence", i+1, cr.Criterion), - Session: sessionResult, - } - } - } - // Convert to assertion results. + return j.buildResult(in, sessionResult, materialized, prompt, criterionResults), nil +} + +func (j *AgentJudge) buildResult(in Input, sessionResult *agent.SessionResult, materialized *MaterializedContext, prompt string, criterionResults []CriterionResult) *Result { assertions := make([]AssertionResult, 0, len(criterionResults)) for _, cr := range criterionResults { assertions = append(assertions, AssertionResult{ @@ -165,21 +172,43 @@ func (j *AgentJudge) Evaluate(ctx context.Context, in Input) (*Result, error) { }) } - // Build result with threshold-based status determination. result := NewResult(assertions, in.TurnsExecuted, in.TurnsTotal) result.JudgeSession = sessionResult + result.JudgeContext = buildContextMetadata(sessionResult, materialized, prompt) + if len(assertions) > 0 && result.Summary.PassRate >= j.PassThreshold { + result.Status = StatusPass + } else if len(assertions) > 0 { + result.Status = StatusFail + } + return result +} - // Agent judge uses pass_threshold to determine status, overriding - // the default all-or-nothing logic in NewResult. - if len(assertions) > 0 { - if result.Summary.PassRate >= j.PassThreshold { - result.Status = StatusPass - } else { - result.Status = StatusFail - } +func buildContextMetadata(sessionResult *agent.SessionResult, materialized *MaterializedContext, prompt string) *ContextMetadata { + metadata := &ContextMetadata{ + Profile: materialized.Manifest.Profile, + MaterializedDir: materialized.Manifest.MaterializedDir, + Manifest: &materialized.Manifest, + PromptBytes: len([]byte(prompt)), + } + if sessionResult != nil && sessionResult.PromptDelivery != nil { + metadata.PromptDelivery = sessionResult.PromptDelivery.Mode + metadata.PromptBytes = sessionResult.PromptDelivery.PromptBytes } + return metadata +} - return result, nil +func validateAgentJudgeResponse(criteria []string, results []CriterionResult) error { + // A short response inflates pass_rate because NewResult uses the returned + // count as the denominator (e.g. 1-of-1 returned = 100% even if 3 were sent). + if len(results) != len(criteria) { + return fmt.Errorf("agent_judge: expected %d criterion results, got %d", len(criteria), len(results)) + } + for i, cr := range results { + if strings.TrimSpace(cr.Evidence) == "" { + return fmt.Errorf("agent_judge: criterion %d %q has empty evidence", i+1, cr.Criterion) + } + } + return nil } // extractJSON finds and parses a JSON object from agent output text. @@ -397,7 +426,7 @@ func canRecoverAgentJudgeResult(err error, sessionResult *agent.SessionResult) b // --------------------------------------------------------------------------- // buildJudgePrompt constructs the system + user prompt for the judge agent. -func buildJudgePrompt(ctx context.Context, criteria []string, finalMessage, workspaceDiff string, transcriptData transcript.Transcript) string { +func buildJudgePrompt(_ context.Context, criteria []string, materialized *MaterializedContext) string { var sb strings.Builder sb.WriteString("You are an expert evaluator for an AI agent skill evaluation.\n") @@ -414,27 +443,29 @@ func buildJudgePrompt(ctx context.Context, criteria []string, finalMessage, work fmt.Fprintf(&sb, "%d. %s\n", i+1, c) } - sb.WriteString("\n## Agent Final Message\n") - sb.WriteString(finalMessage) - sb.WriteString("\n") - - if workspaceDiff != "" { - sb.WriteString("\n## Workspace Diff\n```\n") - sb.WriteString(workspaceDiff) - sb.WriteString("\n```\n") - } - - if transcriptData != nil { - transcriptJSON, err := json.Marshal(transcriptData) - if err != nil { - logging.WarnContextf(ctx, "agent_judge failed to marshal transcript for judge prompt: %v", err) - } else { - transcriptStr := string(transcriptJSON) - if transcriptStr != "" && transcriptStr != "null" { - sb.WriteString("\n## Full Transcript\n") - sb.WriteString(transcriptStr) - sb.WriteString("\n") + if materialized != nil { + sb.WriteString("\n## Review Materials\n") + sb.WriteString("Read files from this table as needed to evaluate the criteria. Evidence must be traceable to the listed material keys or inline excerpts.\n\n") + sb.WriteString("| key | mode | path | bytes | notes |\n") + sb.WriteString("| --- | --- | --- | ---: | --- |\n") + for _, m := range materialized.Materials { + notes := "" + if m.Truncated { + notes = "inline excerpt truncated; full text is at path" } + fmt.Fprintf(&sb, "| %s | %s | %s | %d | %s |\n", materialLabel(m), m.Mode, materialPromptPath(m), m.OriginalBytes, notes) + } + for _, m := range materialized.Materials { + if strings.TrimSpace(m.InlineContent) == "" { + continue + } + fmt.Fprintf(&sb, "\n### Inline Material: %s\n", materialLabel(m)) + if m.Truncated { + sb.WriteString("The excerpt below is truncated; read the full file at the path above if needed.\n") + } + sb.WriteString("```\n") + sb.WriteString(m.InlineContent) + sb.WriteString("\n```\n") } } @@ -455,3 +486,17 @@ func buildJudgePrompt(ctx context.Context, criteria []string, finalMessage, work return sb.String() } + +func materialLabel(m ContextMaterial) string { + if m.Label != "" { + return m.Key + ":" + m.Label + } + return m.Key +} + +func materialPromptPath(m ContextMaterial) string { + if m.RuntimePath != "" { + return m.RuntimePath + } + return m.Path +} diff --git a/internal/judge/agent_judge_test.go b/internal/judge/agent_judge_test.go index 2799dae..9a245e8 100644 --- a/internal/judge/agent_judge_test.go +++ b/internal/judge/agent_judge_test.go @@ -46,6 +46,12 @@ func TestAgentJudge_AllPass(t *testing.T) { if r.Summary.Total != 3 { t.Fatalf("expected 3 assertions, got %d", r.Summary.Total) } + if r.JudgeContext == nil || r.JudgeContext.Manifest == nil { + t.Fatal("expected judge context metadata") + } + if r.JudgeContext.Profile != "standard" { + t.Fatalf("expected standard judge context profile, got %q", r.JudgeContext.Profile) + } } // --------------------------------------------------------------------------- @@ -318,22 +324,47 @@ func TestAgentJudge_CustomThreshold(t *testing.T) { // --------------------------------------------------------------------------- func TestBuildJudgePrompt_ContainsAllParts(t *testing.T) { + materialized := &MaterializedContext{ + Materials: []ContextMaterial{ + { + ContextMaterialManifest: ContextMaterialManifest{ + Key: "final_message", + Mode: "include", + Path: "/tmp/final_message.txt", + OriginalBytes: len("Agent found a bug at line 42"), + }, + InlineContent: "Agent found a bug at line 42", + }, + { + ContextMaterialManifest: ContextMaterialManifest{ + Key: "workspace_diff", + Mode: "file_ref", + Path: "/tmp/workspace.diff", + OriginalBytes: len("diff --git a/main.go"), + }, + }, + { + ContextMaterialManifest: ContextMaterialManifest{ + Key: "transcript", + Mode: "file_ref", + Path: "/tmp/transcript.json", + OriginalBytes: len("review"), + }, + }, + }, + } prompt := buildJudgePrompt( context.Background(), []string{"criterion A", "criterion B"}, - "Agent found a bug at line 42", - "diff --git a/main.go", - transcript.Transcript{ - {Role: transcript.RoleUser, Content: "review"}, - }, + materialized, ) checks := []string{ "criterion A", "criterion B", "Agent found a bug at line 42", - "diff --git a/main.go", - "review", + "/tmp/workspace.diff", + "/tmp/transcript.json", "\"passed\"", "\"evidence\"", } @@ -344,34 +375,38 @@ func TestBuildJudgePrompt_ContainsAllParts(t *testing.T) { } } -func TestBuildJudgePrompt_TranscriptMarshalFailure_LogsAndOmitsErrorFromPrompt(t *testing.T) { - var prompt string - captured := captureLogOutput(t, func() { - prompt = buildJudgePrompt( - context.Background(), - []string{"criterion A"}, - "Agent final message", - "", - transcript.Transcript{ - { - Role: transcript.RoleToolResult, - ToolResult: &transcript.ToolResultInfo{ - Status: "success", - Content: func() {}, - }, - }, - }, - ) - }) +func TestBuildContextMetadata_UsesManifestMaterializedDir(t *testing.T) { + materialized := &MaterializedContext{ + Dir: "/tmp/skill-up/run/context", + Manifest: ContextManifest{ + Profile: "standard", + MaterializedDir: judgeContextArtifactDir, + }, + } - if strings.Contains(prompt, "marshal error") { - t.Fatalf("prompt should not expose marshal error details: %q", prompt) + metadata := buildContextMetadata(nil, materialized, "prompt") + if metadata.MaterializedDir != judgeContextArtifactDir { + t.Fatalf("materialized_dir = %q, want %q", metadata.MaterializedDir, judgeContextArtifactDir) } - if strings.Contains(prompt, "## Full Transcript") { - t.Fatalf("prompt should omit transcript section on marshal failure: %q", prompt) +} + +func TestMaterializeJudgeContext_TranscriptMarshalFailure_ReturnsError(t *testing.T) { + _, err := MaterializeJudgeContext(context.Background(), &mockJudgeTestRuntime{}, nil, Input{ + Transcript: transcript.Transcript{ + { + Role: transcript.RoleToolResult, + ToolResult: &transcript.ToolResultInfo{ + Status: "success", + Content: func() {}, + }, + }, + }, + }, t.TempDir()) + if err == nil { + t.Fatal("expected marshal error") } - if !strings.Contains(captured, "failed to marshal transcript for judge prompt") { - t.Fatalf("expected warning log, got %q", captured) + if !strings.Contains(err.Error(), "marshal transcript") { + t.Fatalf("expected transcript marshal error, got %v", err) } } diff --git a/internal/judge/context_materializer.go b/internal/judge/context_materializer.go new file mode 100644 index 0000000..b625187 --- /dev/null +++ b/internal/judge/context_materializer.go @@ -0,0 +1,505 @@ +package judge + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + pathpkg "path" + "path/filepath" + "strings" + "unicode" + + "github.com/alibaba/skill-up/internal/config" + "github.com/alibaba/skill-up/internal/logging" + "github.com/alibaba/skill-up/internal/runtime" + "github.com/alibaba/skill-up/pkg/transcript" +) + +const ( + judgeContextProfileMinimal = "minimal" + judgeContextProfileStandard = "standard" + + judgeContextModeInclude = "include" + judgeContextModeOmit = "omit" + judgeContextModeTruncate = "truncate" + judgeContextModeFileRef = "file_ref" + + judgeContextGeneratedOmit = "omit" + judgeContextGeneratedIndex = "index" + judgeContextGeneratedInclude = "include" + + defaultJudgeContextMaxBytes = 64 * 1024 + judgeContextArtifactDir = "judge/context" +) + +// MaterializedContext describes the files and inline previews made available +// to an agent_judge invocation. +type MaterializedContext struct { + Dir string + RuntimeDir string + Manifest ContextManifest + Materials []ContextMaterial +} + +// ContextManifest is persisted as manifest.json and copied into reports. +type ContextManifest struct { + Profile string `json:"profile"` + MaterializedDir string `json:"materialized_dir,omitempty"` + RuntimeDir string `json:"runtime_dir,omitempty"` + Materials []ContextMaterialManifest `json:"materials"` +} + +// ContextMaterialManifest records one material's delivery decision. +type ContextMaterialManifest struct { + Key string `json:"key"` + Label string `json:"label,omitempty"` + Mode string `json:"mode"` + Path string `json:"path,omitempty"` + Bytes int `json:"bytes"` + OriginalBytes int `json:"original_bytes,omitempty"` + Truncated bool `json:"truncated,omitempty"` +} + +// ContextMaterial is a prompt-facing material entry. +type ContextMaterial struct { + ContextMaterialManifest + + InlineContent string `json:"-"` + RuntimePath string `json:"-"` +} + +type effectiveJudgeContext struct { + profile string + finalMessageMode string + transcriptMode string + workspaceDiffMode string + generatedFilesMode string + maxBytes int + transcriptMaxTurns int + workspaceDiffMaxLines int + attachments []config.JudgeContextAttachment +} + +// MaterializeJudgeContext writes agent_judge materials to disk and uploads +// readable copies into the runtime workspace. +func MaterializeJudgeContext(ctx context.Context, rt runtime.Runtime, cfg *config.JudgeContextConfig, in Input, artifactDir string) (*MaterializedContext, error) { + effective := resolveJudgeContext(cfg) + hostDir, err := judgeContextHostDir(artifactDir) + if err != nil { + return nil, err + } + if err := os.RemoveAll(hostDir); err != nil { + return nil, fmt.Errorf("clear judge context dir: %w", err) + } + if err := os.MkdirAll(hostDir, 0o755); err != nil { + return nil, fmt.Errorf("create judge context dir: %w", err) + } + + runtimeDir := ".skill-up/judge/context" + if rt != nil && strings.TrimSpace(rt.Workspace()) != "" { + runtimeDir = filepath.Join(rt.Workspace(), runtimeDir) + } + mc := &MaterializedContext{ + Dir: hostDir, + RuntimeDir: runtimeDir, + Manifest: ContextManifest{ + Profile: effective.profile, + MaterializedDir: judgeContextArtifactDir, + }, + } + + if err := materializeText(ctx, rt, mc, "final_message", "final_message.txt", in.FinalMessage, effective.finalMessageMode, effective.maxBytes); err != nil { + return nil, err + } + transcriptJSON, err := marshalTranscript(in.Transcript, effective.transcriptMaxTurns) + if err != nil { + return nil, err + } + if err := materializeText(ctx, rt, mc, "transcript", "transcript.json", transcriptJSON, effective.transcriptMode, effective.maxBytes); err != nil { + return nil, err + } + diff := limitLines(in.WorkspaceDiff, effective.workspaceDiffMaxLines) + if err := materializeText(ctx, rt, mc, "workspace_diff", "workspace.diff", diff, effective.workspaceDiffMode, effective.maxBytes); err != nil { + return nil, err + } + if err := materializeGeneratedFiles(ctx, rt, mc, in.GeneratedFiles, effective.generatedFilesMode, effective.maxBytes); err != nil { + return nil, err + } + if err := materializeAttachments(ctx, rt, mc, effective.attachments, in); err != nil { + return nil, err + } + + manifestPath := filepath.Join(hostDir, "manifest.json") + data, err := json.MarshalIndent(mc.Manifest, "", " ") + if err != nil { + return nil, fmt.Errorf("marshal judge context manifest: %w", err) + } + data = append(data, '\n') + if err := os.WriteFile(manifestPath, data, 0o600); err != nil { + return nil, fmt.Errorf("write judge context manifest: %w", err) + } + if err := uploadRuntimeFile(ctx, rt, manifestPath, filepath.Join(runtimeDir, "manifest.json")); err != nil { + return nil, err + } + + logging.InfoContextf(ctx, "judge context materialized profile=%s dir=%s", effective.profile, hostDir) + return mc, nil +} + +func resolveJudgeContext(cfg *config.JudgeContextConfig) effectiveJudgeContext { + profile := judgeContextProfileStandard + if cfg != nil && cfg.Profile != "" { + profile = cfg.Profile + } + effective := effectiveJudgeContext{ + profile: profile, + maxBytes: defaultJudgeContextMaxBytes, + generatedFilesMode: judgeContextGeneratedIndex, + workspaceDiffMaxLines: 0, + } + switch profile { + case judgeContextProfileMinimal: + effective.finalMessageMode = judgeContextModeTruncate + effective.transcriptMode = judgeContextModeOmit + effective.workspaceDiffMode = judgeContextModeOmit + effective.generatedFilesMode = judgeContextGeneratedOmit + default: + effective.finalMessageMode = judgeContextModeInclude + effective.transcriptMode = judgeContextModeFileRef + effective.workspaceDiffMode = judgeContextModeFileRef + } + if cfg == nil { + return effective + } + if cfg.FinalMessage != "" { + effective.finalMessageMode = cfg.FinalMessage + } + if cfg.Transcript != "" { + effective.transcriptMode = cfg.Transcript + } + if cfg.WorkspaceDiff != "" { + effective.workspaceDiffMode = cfg.WorkspaceDiff + } + if cfg.GeneratedFiles != "" { + effective.generatedFilesMode = cfg.GeneratedFiles + } + if cfg.Limits != nil { + if cfg.Limits.MaxBytes > 0 { + effective.maxBytes = cfg.Limits.MaxBytes + } + effective.transcriptMaxTurns = cfg.Limits.TranscriptMaxTurns + effective.workspaceDiffMaxLines = cfg.Limits.WorkspaceDiffMaxLines + } + effective.attachments = append([]config.JudgeContextAttachment(nil), cfg.Attachments...) + return effective +} + +func judgeContextHostDir(artifactDir string) (string, error) { + if artifactDir == "" { + return os.MkdirTemp("", "skill-up-judge-context-*") + } + return filepath.Join(filepath.Dir(artifactDir), "context"), nil +} + +func materializeText(ctx context.Context, rt runtime.Runtime, mc *MaterializedContext, key, fileName, content, mode string, maxBytes int) error { + if mode == "" { + mode = judgeContextModeFileRef + } + material := ContextMaterial{ + ContextMaterialManifest: ContextMaterialManifest{ + Key: key, + Mode: mode, + Bytes: len([]byte(content)), + OriginalBytes: len([]byte(content)), + }, + } + if strings.TrimSpace(content) == "" && mode != judgeContextModeOmit { + mode = judgeContextModeOmit + material.Mode = mode + } + if mode == judgeContextModeOmit { + material.Bytes = 0 + material.OriginalBytes = 0 + mc.Manifest.Materials = append(mc.Manifest.Materials, material.ContextMaterialManifest) + return nil + } + + if mode == judgeContextModeInclude && len([]byte(content)) > maxBytes { + mode = judgeContextModeFileRef + material.Mode = mode + } + runtimePath, manifestPath, err := writeMaterialFile(ctx, rt, mc, fileName, content) + if err != nil { + return err + } + material.Path = manifestPath + material.RuntimePath = runtimePath + switch mode { + case judgeContextModeInclude: + material.InlineContent = content + case judgeContextModeTruncate: + material.InlineContent = truncateBytes(content, maxBytes) + material.Bytes = len([]byte(material.InlineContent)) + material.Truncated = material.Bytes < material.OriginalBytes + case judgeContextModeFileRef: + default: + return fmt.Errorf("unsupported judge context mode %q for %s", mode, key) + } + mc.Manifest.Materials = append(mc.Manifest.Materials, material.ContextMaterialManifest) + mc.Materials = append(mc.Materials, material) + return nil +} + +func writeMaterialFile(ctx context.Context, rt runtime.Runtime, mc *MaterializedContext, fileName, content string) (runtimePath string, manifestPath string, err error) { + safeName, err := safeMaterialRelPath(fileName) + if err != nil { + return "", "", err + } + hostPath, err := safeMaterialHostPath(mc.Dir, safeName) + if err != nil { + return "", "", err + } + if err := os.MkdirAll(filepath.Dir(hostPath), 0o755); err != nil { + return "", "", fmt.Errorf("create material dir: %w", err) + } + // #nosec G703 -- hostPath is constrained by safeMaterialRelPath and safeMaterialHostPath to stay under mc.Dir. + if err := os.WriteFile(hostPath, []byte(content), 0o600); err != nil { + return "", "", fmt.Errorf("write material %s: %w", fileName, err) + } + runtimePath = materialRuntimePath(mc, safeName) + if err := uploadRuntimeFile(ctx, rt, hostPath, runtimePath); err != nil { + return "", "", err + } + return runtimePath, materialManifestPath(safeName), nil +} + +func copyMaterialFile(ctx context.Context, rt runtime.Runtime, mc *MaterializedContext, fileName, sourcePath string) (runtimePath string, manifestPath string, bytes int, err error) { + safeName, err := safeMaterialRelPath(fileName) + if err != nil { + return "", "", 0, err + } + hostPath, err := safeMaterialHostPath(mc.Dir, safeName) + if err != nil { + return "", "", 0, err + } + if err := os.MkdirAll(filepath.Dir(hostPath), 0o755); err != nil { + return "", "", 0, fmt.Errorf("create material dir: %w", err) + } + // #nosec G304 -- sourcePath is resolved by resolveAttachmentPath to stay within skill/workspace roots. + src, err := os.Open(sourcePath) + if err != nil { + return "", "", 0, fmt.Errorf("read judge context attachment %q: %w", sourcePath, err) + } + // #nosec G304 -- hostPath is constrained by safeMaterialRelPath and safeMaterialHostPath to stay under mc.Dir. + dst, err := os.OpenFile(hostPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) + if err != nil { + if closeErr := src.Close(); closeErr != nil { + return "", "", 0, fmt.Errorf("close judge context attachment %q: %w", sourcePath, closeErr) + } + return "", "", 0, fmt.Errorf("write material %s: %w", fileName, err) + } + copied, copyErr := io.Copy(dst, src) + dstCloseErr := dst.Close() + srcCloseErr := src.Close() + if copyErr != nil { + return "", "", 0, fmt.Errorf("copy material %s: %w", fileName, copyErr) + } + if dstCloseErr != nil { + return "", "", 0, fmt.Errorf("close material %s: %w", fileName, dstCloseErr) + } + if srcCloseErr != nil { + return "", "", 0, fmt.Errorf("close judge context attachment %q: %w", sourcePath, srcCloseErr) + } + if copied > int64(^uint(0)>>1) { + return "", "", 0, fmt.Errorf("material %s is too large", fileName) + } + runtimePath = materialRuntimePath(mc, safeName) + if err := uploadRuntimeFile(ctx, rt, hostPath, runtimePath); err != nil { + return "", "", 0, err + } + return runtimePath, materialManifestPath(safeName), int(copied), nil +} + +func materialRuntimePath(mc *MaterializedContext, safeName string) string { + return filepath.Join(mc.RuntimeDir, filepath.ToSlash(safeName)) +} + +func materialManifestPath(safeName string) string { + return pathpkg.Join(judgeContextArtifactDir, filepath.ToSlash(safeName)) +} + +func safeMaterialRelPath(fileName string) (string, error) { + if fileName == "" || filepath.IsAbs(fileName) { + return "", fmt.Errorf("invalid judge context material path %q", fileName) + } + clean := filepath.Clean(fileName) + if clean == "." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) || clean == ".." { + return "", fmt.Errorf("invalid judge context material path %q", fileName) + } + return clean, nil +} + +func safeMaterialHostPath(root, rel string) (string, error) { + hostPath := filepath.Join(root, rel) + cleanRoot := filepath.Clean(root) + cleanHostPath := filepath.Clean(hostPath) + if !isWithin(cleanHostPath, cleanRoot) { + return "", fmt.Errorf("judge context material path escapes context dir: %q", rel) + } + return cleanHostPath, nil +} + +func uploadRuntimeFile(ctx context.Context, rt runtime.Runtime, hostPath, runtimePath string) error { + if rt == nil { + return nil + } + if err := rt.UploadFile(ctx, hostPath, runtimePath); err != nil { + return fmt.Errorf("upload judge context file %s: %w", filepath.Base(hostPath), err) + } + return nil +} + +func materializeGeneratedFiles(ctx context.Context, rt runtime.Runtime, mc *MaterializedContext, files []string, mode string, maxBytes int) error { + if mode == "" { + mode = judgeContextGeneratedIndex + } + if mode == judgeContextGeneratedOmit || len(files) == 0 { + mc.Manifest.Materials = append(mc.Manifest.Materials, ContextMaterialManifest{ + Key: "generated_files", + Mode: judgeContextGeneratedOmit, + }) + return nil + } + content := strings.Join(files, "\n") + if content != "" { + content += "\n" + } + textMode := judgeContextModeFileRef + if mode == judgeContextGeneratedInclude && len([]byte(content)) <= maxBytes { + textMode = judgeContextModeInclude + } + return materializeText(ctx, rt, mc, "generated_files", "generated_files.txt", content, textMode, maxBytes) +} + +func materializeAttachments(ctx context.Context, rt runtime.Runtime, mc *MaterializedContext, attachments []config.JudgeContextAttachment, in Input) error { + if len(attachments) == 0 { + return nil + } + for i, attachment := range attachments { + src, err := resolveAttachmentPath(attachment.Path, in) + if err != nil { + return err + } + label := attachment.Label + if label == "" { + label = strings.TrimSuffix(filepath.Base(src), filepath.Ext(src)) + } + name := fmt.Sprintf("%02d-%s%s", i+1, safeMaterialName(label), filepath.Ext(src)) + runtimePath, manifestPath, bytes, err := copyMaterialFile(ctx, rt, mc, filepath.Join("attachments", name), src) + if err != nil { + return err + } + material := ContextMaterial{ + ContextMaterialManifest: ContextMaterialManifest{ + Key: "attachment", + Label: label, + Mode: judgeContextModeFileRef, + Path: manifestPath, + Bytes: bytes, + OriginalBytes: bytes, + }, + RuntimePath: runtimePath, + } + mc.Manifest.Materials = append(mc.Manifest.Materials, material.ContextMaterialManifest) + mc.Materials = append(mc.Materials, material) + } + return nil +} + +func resolveAttachmentPath(raw string, in Input) (string, error) { + path := raw + if !filepath.IsAbs(path) { + path = filepath.Join(in.SkillDir, path) + } + abs, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("resolve judge context attachment %q: %w", raw, err) + } + for _, root := range []string{in.SkillDir, in.WorkspacePath} { + if root == "" { + continue + } + if isWithin(abs, root) { + return abs, nil + } + } + if in.SkillDir == "" && in.WorkspacePath == "" { + return abs, nil + } + return "", fmt.Errorf("judge context attachment %q must stay within skill_dir or workspace", raw) +} + +func isWithin(path, root string) bool { + absRoot, err := filepath.Abs(root) + if err != nil { + return false + } + rel, err := filepath.Rel(absRoot, path) + if err != nil { + return false + } + return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))) +} + +func marshalTranscript(trans transcript.Transcript, maxTurns int) (string, error) { + if len(trans) == 0 { + return "", nil + } + if maxTurns > 0 && len(trans) > maxTurns { + trans = trans[len(trans)-maxTurns:] + } + data, err := json.MarshalIndent(trans, "", " ") + if err != nil { + return "", fmt.Errorf("marshal transcript: %w", err) + } + return string(data), nil +} + +func limitLines(s string, maxLines int) string { + if maxLines <= 0 || s == "" { + return s + } + lines := strings.SplitAfter(s, "\n") + if len(lines) <= maxLines { + return s + } + return strings.Join(lines[:maxLines], "") +} + +func truncateBytes(s string, maxBytes int) string { + if maxBytes <= 0 || len([]byte(s)) <= maxBytes { + return s + } + b := []byte(s) + return string(b[:maxBytes]) +} + +func safeMaterialName(s string) string { + var b strings.Builder + for _, r := range strings.ToLower(s) { + if unicode.IsLetter(r) || unicode.IsDigit(r) { + b.WriteRune(r) + continue + } + if b.Len() > 0 { + b.WriteByte('-') + } + } + name := strings.Trim(b.String(), "-") + if name == "" { + return "attachment" + } + return name +} diff --git a/internal/judge/context_materializer_test.go b/internal/judge/context_materializer_test.go new file mode 100644 index 0000000..944b79a --- /dev/null +++ b/internal/judge/context_materializer_test.go @@ -0,0 +1,186 @@ +package judge + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/alibaba/skill-up/internal/config" + "github.com/alibaba/skill-up/pkg/transcript" +) + +func TestMaterializeJudgeContext_StandardProfileUsesFileRefs(t *testing.T) { + artifactDir := filepath.Join(t.TempDir(), "judge", "run") + if err := os.MkdirAll(artifactDir, 0o755); err != nil { + t.Fatalf("mkdir artifact dir: %v", err) + } + + mc, err := MaterializeJudgeContext(context.Background(), &mockJudgeTestRuntime{}, nil, Input{ + FinalMessage: "final answer", + WorkspaceDiff: "diff --git a/a b/a\n", + Transcript: transcript.Transcript{ + {Role: transcript.RoleUser, Content: "prompt", Turn: 1}, + }, + }, artifactDir) + if err != nil { + t.Fatalf("MaterializeJudgeContext returned error: %v", err) + } + if mc.Manifest.Profile != "standard" { + t.Fatalf("profile = %q, want standard", mc.Manifest.Profile) + } + if mc.Manifest.MaterializedDir != judgeContextArtifactDir { + t.Fatalf("materialized_dir = %q, want %q", mc.Manifest.MaterializedDir, judgeContextArtifactDir) + } + if mc.Manifest.RuntimeDir != "" { + t.Fatalf("runtime_dir should be omitted from persisted manifest, got %q", mc.Manifest.RuntimeDir) + } + assertMaterialMode(t, mc.Manifest.Materials, "final_message", "include") + assertMaterialMode(t, mc.Manifest.Materials, "transcript", "file_ref") + assertMaterialMode(t, mc.Manifest.Materials, "workspace_diff", "file_ref") + assertMaterialPath(t, mc.Manifest.Materials, "transcript", "judge/context/transcript.json") + if _, err := os.Stat(filepath.Join(mc.Dir, "transcript.json")); err != nil { + t.Fatalf("transcript material missing: %v", err) + } + if _, err := os.Stat(filepath.Join(mc.Dir, "workspace.diff")); err != nil { + t.Fatalf("workspace diff material missing: %v", err) + } +} + +func TestMaterializeJudgeContext_MinimalProfileOmitsLargeMaterials(t *testing.T) { + mc, err := MaterializeJudgeContext(context.Background(), &mockJudgeTestRuntime{}, &config.JudgeContextConfig{ + Profile: "minimal", + Limits: &config.JudgeContextLimits{MaxBytes: 5}, + }, Input{ + FinalMessage: "0123456789", + WorkspaceDiff: "diff", + Transcript: transcript.Transcript{ + {Role: transcript.RoleUser, Content: "prompt", Turn: 1}, + }, + }, filepath.Join(t.TempDir(), "judge", "run")) + if err != nil { + t.Fatalf("MaterializeJudgeContext returned error: %v", err) + } + assertMaterialMode(t, mc.Manifest.Materials, "final_message", "truncate") + assertMaterialMode(t, mc.Manifest.Materials, "transcript", "omit") + assertMaterialMode(t, mc.Manifest.Materials, "workspace_diff", "omit") + var final ContextMaterial + for _, m := range mc.Materials { + if m.Key == "final_message" { + final = m + } + } + if final.InlineContent != "01234" || !final.Truncated { + t.Fatalf("expected truncated final message, got content=%q truncated=%v", final.InlineContent, final.Truncated) + } +} + +func TestMaterializeJudgeContext_IncludeAutoDowngradesToFileRef(t *testing.T) { + mc, err := MaterializeJudgeContext(context.Background(), &mockJudgeTestRuntime{}, &config.JudgeContextConfig{ + Transcript: "include", + Limits: &config.JudgeContextLimits{MaxBytes: 10}, + }, Input{ + FinalMessage: "ok", + Transcript: transcript.Transcript{ + {Role: transcript.RoleUser, Content: "this is a long prompt", Turn: 1}, + }, + }, filepath.Join(t.TempDir(), "judge", "run")) + if err != nil { + t.Fatalf("MaterializeJudgeContext returned error: %v", err) + } + assertMaterialMode(t, mc.Manifest.Materials, "transcript", "file_ref") + for _, material := range mc.Materials { + if material.Key == "transcript" && material.InlineContent != "" { + t.Fatalf("auto-downgraded transcript should not be inlined") + } + } +} + +func TestMaterializeJudgeContext_AttachmentCopy(t *testing.T) { + skillDir := t.TempDir() + attachmentBytes := []byte{0xff, 0x00, 'a', '\n'} + if err := os.WriteFile(filepath.Join(skillDir, "diff-result.bin"), attachmentBytes, 0o600); err != nil { + t.Fatalf("write attachment: %v", err) + } + + mc, err := MaterializeJudgeContext(context.Background(), &mockJudgeTestRuntime{}, &config.JudgeContextConfig{ + Profile: "minimal", + Attachments: []config.JudgeContextAttachment{ + {Path: "diff-result.bin", Label: "diff_result"}, + }, + }, Input{ + SkillDir: skillDir, + FinalMessage: "ok", + WorkspacePath: t.TempDir(), + }, filepath.Join(t.TempDir(), "judge", "run")) + if err != nil { + t.Fatalf("MaterializeJudgeContext returned error: %v", err) + } + assertMaterialMode(t, mc.Manifest.Materials, "attachment", "file_ref") + attachmentPath := filepath.Join(mc.Dir, "attachments", "01-diff-result.bin") + copiedBytes, err := os.ReadFile(attachmentPath) + if err != nil { + t.Fatalf("attachment copy missing: %v", err) + } + if !bytes.Equal(copiedBytes, attachmentBytes) { + t.Fatalf("attachment bytes changed: got %v want %v", copiedBytes, attachmentBytes) + } + assertMaterialPath(t, mc.Manifest.Materials, "attachment", "judge/context/attachments/01-diff-result.bin") +} + +func TestBuildJudgePrompt_StandardProfileDoesNotInlineLargeTranscript(t *testing.T) { + // ~128 KB transcript body, far above any inline limit. Under the default + // (standard) profile it must be referenced by path, never inlined, so the + // judge prompt stays small and cannot trigger ARG_MAX (proposal R1/R4). + marker := strings.Repeat("TRANSCRIPT-BODY-", 8000) + mc, err := MaterializeJudgeContext(context.Background(), &mockJudgeTestRuntime{}, nil, Input{ + FinalMessage: "done", + Transcript: transcript.Transcript{ + {Role: transcript.RoleAssistant, Content: marker, Turn: 1}, + }, + }, filepath.Join(t.TempDir(), "judge", "run")) + if err != nil { + t.Fatalf("MaterializeJudgeContext returned error: %v", err) + } + + assertMaterialMode(t, mc.Manifest.Materials, "transcript", "file_ref") + + prompt := buildJudgePrompt(context.Background(), []string{"criterion A"}, mc) + if strings.Contains(prompt, marker) { + t.Fatal("standard-profile judge prompt must not inline the full transcript body") + } + if len(prompt) > 8*1024 { + t.Fatalf("judge prompt too large: %d bytes (transcript should be referenced by path)", len(prompt)) + } + if _, err := os.Stat(filepath.Join(mc.Dir, "transcript.json")); err != nil { + t.Fatalf("transcript file reference missing: %v", err) + } +} + +func assertMaterialMode(t *testing.T, materials []ContextMaterialManifest, key, mode string) { + t.Helper() + for _, material := range materials { + if material.Key == key { + if material.Mode != mode { + t.Fatalf("%s mode = %q, want %q", key, material.Mode, mode) + } + return + } + } + t.Fatalf("material %q not found in %#v", key, materials) +} + +func assertMaterialPath(t *testing.T, materials []ContextMaterialManifest, key, want string) { + t.Helper() + for _, material := range materials { + if material.Key == key { + if material.Path != want { + t.Fatalf("%s path = %q, want %q", key, material.Path, want) + } + return + } + } + t.Fatalf("material %q not found in %#v", key, materials) +} diff --git a/internal/judge/factory.go b/internal/judge/factory.go index 79de5a9..45e0ffd 100644 --- a/internal/judge/factory.go +++ b/internal/judge/factory.go @@ -38,7 +38,7 @@ func NewJudge(cfg config.JudgeConfig, ag agent.Agent, rt runtime.Runtime) (Judge if ag == nil { return nil, errors.New("agent_judge requires an Agent") } - return NewAgentJudge(ag, rt, cfg.Model, cfg.Criteria, cfg.PassThreshold, derefInt(cfg.TimeoutSeconds)), nil + return NewAgentJudgeWithContext(ag, rt, cfg.Model, cfg.Criteria, cfg.PassThreshold, cfg.Context, derefInt(cfg.TimeoutSeconds)), nil case "": // No judge configured — return nil, caller should handle @@ -79,8 +79,80 @@ func MergeJudgeConfig(global, caseLevel config.JudgeConfig) config.JudgeConfig { if merged.TimeoutSeconds == nil { merged.TimeoutSeconds = global.TimeoutSeconds } + merged.Context = MergeJudgeContextConfig(global.Context, caseLevel.Context) return merged } // No case-level judge, use global return global } + +// MergeJudgeContextConfig merges case-level judge.context over eval-level +// judge.context. Unset fields inherit from the global context. +func MergeJudgeContextConfig(global, caseLevel *config.JudgeContextConfig) *config.JudgeContextConfig { + if global == nil && caseLevel == nil { + return nil + } + if global == nil { + return cloneJudgeContextConfig(caseLevel) + } + if caseLevel == nil { + return cloneJudgeContextConfig(global) + } + + merged := cloneJudgeContextConfig(global) + if caseLevel.Profile != "" { + merged.Profile = caseLevel.Profile + } + if caseLevel.FinalMessage != "" { + merged.FinalMessage = caseLevel.FinalMessage + } + if caseLevel.Transcript != "" { + merged.Transcript = caseLevel.Transcript + } + if caseLevel.WorkspaceDiff != "" { + merged.WorkspaceDiff = caseLevel.WorkspaceDiff + } + if caseLevel.GeneratedFiles != "" { + merged.GeneratedFiles = caseLevel.GeneratedFiles + } + merged.Limits = mergeJudgeContextLimits(global.Limits, caseLevel.Limits) + if caseLevel.Attachments != nil { + merged.Attachments = append([]config.JudgeContextAttachment(nil), caseLevel.Attachments...) + } + return merged +} + +func mergeJudgeContextLimits(global, caseLevel *config.JudgeContextLimits) *config.JudgeContextLimits { + if global == nil && caseLevel == nil { + return nil + } + var limits config.JudgeContextLimits + if global != nil { + limits = *global + } + if caseLevel != nil { + if caseLevel.MaxBytes > 0 { + limits.MaxBytes = caseLevel.MaxBytes + } + if caseLevel.TranscriptMaxTurns > 0 { + limits.TranscriptMaxTurns = caseLevel.TranscriptMaxTurns + } + if caseLevel.WorkspaceDiffMaxLines > 0 { + limits.WorkspaceDiffMaxLines = caseLevel.WorkspaceDiffMaxLines + } + } + return &limits +} + +func cloneJudgeContextConfig(in *config.JudgeContextConfig) *config.JudgeContextConfig { + if in == nil { + return nil + } + out := *in + if in.Limits != nil { + limits := *in.Limits + out.Limits = &limits + } + out.Attachments = append([]config.JudgeContextAttachment(nil), in.Attachments...) + return &out +} diff --git a/internal/judge/factory_test.go b/internal/judge/factory_test.go index 2654ed1..068467c 100644 --- a/internal/judge/factory_test.go +++ b/internal/judge/factory_test.go @@ -142,6 +142,26 @@ func TestNewJudge_AgentJudge_PropagatesTimeoutSeconds(t *testing.T) { } } +func TestNewJudge_AgentJudge_PropagatesContext(t *testing.T) { + cfg := config.JudgeConfig{ + Type: "agent_judge", + Model: "test-model", + Criteria: []string{"c1"}, + Context: &config.JudgeContextConfig{Profile: "minimal"}, + } + j, err := NewJudge(cfg, &mockJudgeTestAgent{}, &mockJudgeTestRuntime{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + aj, ok := j.(*AgentJudge) + if !ok { + t.Fatalf("expected *AgentJudge, got %T", j) + } + if aj.Context == nil || aj.Context.Profile != "minimal" { + t.Fatalf("expected context profile minimal, got %#v", aj.Context) + } +} + func TestNewJudge_AgentJudge_NilTimeoutSecondsDefaultsToZero(t *testing.T) { cfg := config.JudgeConfig{ Type: "agent_judge", @@ -233,6 +253,58 @@ func TestMergeJudgeConfig_CaseTimeoutOverridesGlobal(t *testing.T) { } } +func TestMergeJudgeConfig_CaseContextOverridesGlobalFields(t *testing.T) { + global := config.JudgeConfig{ + Type: "agent_judge", + Model: "global-model", + Criteria: []string{"global"}, + Context: &config.JudgeContextConfig{ + Profile: "standard", + Transcript: "file_ref", + WorkspaceDiff: "file_ref", + Limits: &config.JudgeContextLimits{ + MaxBytes: 100, + WorkspaceDiffMaxLines: 50, + }, + }, + } + caseLevel := config.JudgeConfig{ + Type: "agent_judge", + Criteria: []string{"case"}, + Context: &config.JudgeContextConfig{ + Profile: "minimal", + FinalMessage: "truncate", + Limits: &config.JudgeContextLimits{TranscriptMaxTurns: 20}, + }, + } + + merged := MergeJudgeConfig(global, caseLevel) + if merged.Context == nil { + t.Fatal("expected merged context") + } + if merged.Context.Profile != "minimal" { + t.Fatalf("profile = %q, want minimal", merged.Context.Profile) + } + if merged.Context.Transcript != "file_ref" { + t.Fatalf("transcript = %q, want inherited file_ref", merged.Context.Transcript) + } + if merged.Context.FinalMessage != "truncate" { + t.Fatalf("final_message = %q, want truncate", merged.Context.FinalMessage) + } + if merged.Context.Limits == nil { + t.Fatalf("limits not inherited: %#v", merged.Context.Limits) + } + if merged.Context.Limits.MaxBytes != 100 { + t.Fatalf("limits.max_bytes = %d, want inherited 100", merged.Context.Limits.MaxBytes) + } + if merged.Context.Limits.TranscriptMaxTurns != 20 { + t.Fatalf("limits.transcript_max_turns = %d, want case override 20", merged.Context.Limits.TranscriptMaxTurns) + } + if merged.Context.Limits.WorkspaceDiffMaxLines != 50 { + t.Fatalf("limits.workspace_diff_max_lines = %d, want inherited 50", merged.Context.Limits.WorkspaceDiffMaxLines) + } +} + func TestMergeJudgeConfig_NoCaseLevel(t *testing.T) { global := config.JudgeConfig{ Type: "rule_based", diff --git a/internal/judge/judge.go b/internal/judge/judge.go index 75d852b..571823e 100644 --- a/internal/judge/judge.go +++ b/internal/judge/judge.go @@ -135,6 +135,20 @@ type Result struct { // session (e.g. agent_judge). It is not part of grading.json; the evaluator // uses it to download judge-run artifacts the same way as the main agent run. JudgeSession *agent.SessionResult `json:"-"` + + // JudgeContext records how agent_judge materialized and delivered review + // materials. It is omitted for deterministic judges and older results. + JudgeContext *ContextMetadata `json:"judge_context,omitempty"` +} + +// ContextMetadata is report-facing metadata for agent_judge context +// materialization and prompt delivery. +type ContextMetadata struct { + Profile string `json:"profile"` + MaterializedDir string `json:"materialized_dir,omitempty"` + Manifest *ContextManifest `json:"manifest,omitempty"` + PromptDelivery string `json:"prompt_delivery,omitempty"` + PromptBytes int `json:"prompt_bytes,omitempty"` } // SessionResultError preserves a judge-side session result when evaluation fails. diff --git a/internal/report/grading.go b/internal/report/grading.go index d646488..2d8f02a 100644 --- a/internal/report/grading.go +++ b/internal/report/grading.go @@ -26,6 +26,7 @@ import ( type AnthropicGrading struct { Expectations []AnthropicExpectation `json:"expectations"` Summary AnthropicSummary `json:"summary"` + JudgeContext *judge.ContextMetadata `json:"judge_context,omitempty"` } // AnthropicExpectation is a single expectation result in the Anthropic format. @@ -76,6 +77,7 @@ func ConvertToAnthropicGrading(result *judge.Result) *AnthropicGrading { Total: result.Summary.Total, PassRate: result.Summary.PassRate, }, + JudgeContext: result.JudgeContext, } } diff --git a/internal/report/grading_test.go b/internal/report/grading_test.go index 2b5dca3..df1590e 100644 --- a/internal/report/grading_test.go +++ b/internal/report/grading_test.go @@ -63,6 +63,26 @@ func TestConvertToAnthropicGrading(t *testing.T) { t.Errorf("expected 0 expectations, got %d", len(grading.Expectations)) } }) + + t.Run("judge context metadata", func(t *testing.T) { + t.Parallel() + result := judge.NewResult([]judge.AssertionResult{ + {Text: "check", Passed: true, Evidence: "ok"}, + }, 1, 1) + result.JudgeContext = &judge.ContextMetadata{ + Profile: "minimal", + PromptDelivery: "file", + PromptBytes: 512, + } + + grading := ConvertToAnthropicGrading(result) + if grading.JudgeContext == nil { + t.Fatal("expected judge context metadata") + } + if grading.JudgeContext.Profile != "minimal" || grading.JudgeContext.PromptDelivery != "file" { + t.Fatalf("unexpected judge context: %#v", grading.JudgeContext) + } + }) } func TestConvertToAnthropicGrading_JSONFormat(t *testing.T) { diff --git a/proposals/0004-agent-judge-context-delivery.md b/proposals/0004-agent-judge-context-delivery.md new file mode 100644 index 0000000..d75fd96 --- /dev/null +++ b/proposals/0004-agent-judge-context-delivery.md @@ -0,0 +1,571 @@ +--- +title: Agent Judge Context Delivery and Scale Control +authors: + - "kongtang" +creation-date: 2026-07-07 +last-updated: 2026-07-07 +status: draft +--- + +# SUP-0004: Agent Judge Context Delivery and Scale Control + +Language: English | [中文](zh/0004-agent-judge-context-delivery.md) + + +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Requirements](#requirements) +- [Proposal](#proposal) + - [User Scenario Quick Reference](#user-scenario-quick-reference) + - [Schema Shape](#schema-shape) + - [Prompt Delivery (All Agents)](#prompt-delivery-all-agents) + - [Context Materialization (`agent_judge`)](#context-materialization-agent_judge) + - [Runtime Behavior](#runtime-behavior) + - [Notes/Constraints/Caveats](#notesconstraintscaveats) + - [Risks and Mitigations](#risks-and-mitigations) +- [Design Details](#design-details) + - [Schema Changes](#schema-changes) + - [PromptDelivery](#promptdelivery) + - [ContextMaterializer](#contextmaterializer) + - [AgentJudge Changes](#agentjudge-changes) + - [Evaluator Changes](#evaluator-changes) + - [Observability](#observability) + - [Documentation and Templates](#documentation-and-templates) +- [Test Plan](#test-plan) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) +- [Infrastructure Needed](#infrastructure-needed) +- [Upgrade & Migration Strategy](#upgrade--migration-strategy) + + +## Summary + +skill-up's `agent_judge` currently inlines the full agent `transcript`, `final_message`, and `workspace_diff` into a single judge prompt string, then passes that string to Agent Engines such as Claude Code through `bash -c 'claude -p ""'`. Long-running cases with many tool calls and large workspace diffs can exceed the OS `ARG_MAX` limit and fail with `fork/exec /usr/bin/bash: argument list too long`. + +This proposal introduces two complementary layers: + +1. **Prompt delivery (all agents)**: automatically switch large prompts to file or stdin delivery instead of argv embedding. +2. **`judge.context` (agent_judge only)**: let authors declare which evaluation materials to materialize, reference, omit, or truncate via profiles `minimal` and `standard`. + +The design aligns `agent_judge` with the existing `script_judge` pattern of passing artifacts by path, while preserving criteria + evidence JSON grading. + +## Motivation + +Production CI for a long-running skill eval (a high-load case) reproduced the failure below after the main agent completed and the judge phase started: + +```text +claude-code run failed: fork/exec /usr/bin/bash: argument list too long +agent_judge failed to parse agent output: no valid JSON found in agent output +``` + +The root cause is two-layered: + +| Layer | Mechanism | Effect | +| --- | --- | --- | +| Delivery | `internal/agent/claude_code.go` → `buildClaudePrintCmd` | Entire instruction is shell-quoted into argv | +| Content | `internal/judge/agent_judge.go` → `buildJudgePrompt` | Inlines transcript JSON, workspace diff, and final message | + +For long cases (`timeout_seconds: 3600`, `max_turns: 50`), transcript size alone can reach megabytes. `agent_judge` also collects workspace git diff by default (`judgeNeedsWorkspaceDiff` returns true for all `agent_judge` configs), which can be large when the workspace contains full application checkouts. + +A single boolean such as `include_transcript: false` would help one scenario but would not solve: + +1. Large `workspace_diff` independent of transcript. +2. Large case prompts for the main agent (same argv delivery path). +3. Future common cases: multi-turn evals, code-review skills, repository-change benchmarks. +4. Lack of observability when judge context is truncated or downgraded. + +### Goals + +1. **Eliminate ARG_MAX class failures** for agent and judge invocations under normal large-eval workloads. +2. **Configurable judge context**: control material scope via `judge.context` (profile / attachments); **criteria style stays unchanged**. +3. **Safe defaults**: new evals should not silently inline megabyte-scale context into argv. +4. **Alignment with script_judge**: materialize large artifacts to disk and reference them by path. +5. **Observability**: reports record materialization mode, sizes, truncation, and prompt delivery mode. +6. **Backward compatibility path**: existing short-prompt evals keep current inline behavior. + +### Non-Goals + +1. **No change to `rule_based` semantics**: in-memory transcript assertions remain unchanged. +2. **No replacement of `agent_judge` with `script_judge`**: this proposal keeps LLM grading with criteria + evidence. +3. **No direct HTTP/API judge path**: engines continue to run through existing CLI adapters in MVP. +4. **No automatic criteria inference in MVP**: profile selection remains explicit; smart recommendations are a follow-up. +5. **No cross-case sharing of judge context files**: each case variant keeps its own `judge/context/` directory. + +## Requirements + +### Must Have + +| ID | Requirement | Acceptance Criteria | +| --- | --- | --- | +| R1 | Large prompt delivery | Prompts above threshold use file or stdin delivery; `claude_code` no longer fails with `argument list too long` on multi-MB judge prompts | +| R2 | `judge.context` schema | `JudgeConfig` accepts `context` with `profile` and per-field modes | +| R3 | Context materialization | `agent_judge` writes transcript/diff/final message/attachments under `judge/context/` when configured | +| R4 | Short judge prompt | Default `standard` profile produces a judge prompt bounded by a fixed small size (path references, not full inline JSON) | +| R5 | Auto downgrade | `include` mode still respects `limits.max_bytes` and downgrades to `file_ref` when exceeded | +| R6 | Report metadata | `grading.json` / `result.json` includes `judge_context` manifest | +| R7 | Backward compatibility | Short-prompt evals without `judge.context` continue to pass existing tests | + +### Should Have + +| ID | Requirement | Acceptance Criteria | +| --- | --- | --- | +| S1 | Profile presets | `minimal` and `standard` documented with clear semantics | +| S2 | `attachments` support | Authors can attach business artifacts (for example `diff-result.json`) by path | +| S3 | Engine parity | `codex`, `qodercli`, `qwen_code` use the same `PromptDelivery` helper | +| S4 | Writing guide updates | English and Chinese eval writing guides include `judge.context` examples | + +### Nice to Have + +| ID | Requirement | Acceptance Criteria | +| --- | --- | --- | +| N1 | ARG_MAX probe | Optional pre-exec size check with warn log and forced file mode | +| N2 | `skill-up validate` hints | Suggest `profile: minimal` when `attachments` are configured but the profile still inlines large transcript | +| N3 | Shared materialize package | `script_judge` and `agent_judge` reuse the same artifact writer | + +## Proposal + +### User Scenario Quick Reference + +#### Scenario 1: Long Repository-Change Benchmark (`profile: minimal`) + +The judge primarily relies on script outputs and diff files, not conversation history: + +```yaml +judge: + type: agent_judge + model: anthropic/claude-sonnet-4-6 + context: + profile: minimal + attachments: + - path: evals/fixtures/artifacts/diff-result.json + label: diff_result + criteria: + - "Determine whether code changes meet the expected rules." + - "If results match expected after filtering temp dirs, pass with verifiable evidence." + pass_threshold: 1 +``` + +Key points: + +- Transcript and workspace diff are omitted from the judge prompt. +- Business evidence is materialized via `attachments`; **paths are auto-injected into the judge materials table**—authors do not declare file paths in criteria. +- Judge prompt stays small; the judge agent decides which attachments to read based on criteria. + +#### Scenario 2: Code Review Skill (`profile: standard`, default) + +```yaml +judge: + type: agent_judge + model: anthropic/claude-sonnet-4-6 + context: + profile: standard + criteria: + - "Identified real bugs with accurate locations" + - "Did not false-positive correct code" +``` + +Effective behavior: + +- `transcript.json`, `workspace.diff`, and `final_message.txt` are written under `judge/context/`. +- The framework **auto-injects a materials table** (paths, sizes, modes) into the judge prompt; **criteria are written the same as today**—no need to mention filenames. +- The judge agent opens whichever files it needs based on criteria when writing evidence. +- Looser truncation or retention can be tuned under `standard` via `limits` and per-field overrides (no extra profile). + +### Per-Field Delivery Modes + +`final_message`, `transcript`, `workspace_diff`, and similar fields can each override the profile default: + +| Mode | In judge prompt | On disk | Purpose | +| --- | --- | --- | --- | +| `include` | Full text inlined | Optional mirror | Short material; judge should see it immediately | +| `file_ref` | Path/reference only, no full body | Full file required | Large material; avoids blowing up prompt / argv (`standard` default for transcript and diff) | +| `omit` | Not present | Not written | Material not needed for grading (`minimal` default for transcript and diff) | +| `truncate` | Inline summary + "full text at path" | Full version written | Preview in prompt, full text on disk (`minimal` default for `final_message`) | + +**Safety net**: even with `include`, the framework auto-downgrades to `file_ref` when a segment exceeds `limits.max_bytes`. + +`generated_files` uses similar semantics: `omit`, `index` (paths only), or `include` (inline content). + +### Author Experience: Transparent Material Delivery + +Previously, `agent_judge` **inlined** transcript, diff, and related materials into the judge prompt. Authors only wrote `criteria` and did not think about how materials were delivered. + +This proposal **changes delivery only, not the author mental model**: + +1. The framework materializes materials per `profile` / per-field settings and **auto-injects a materials table** into the judge system prompt (field name, path, size, truncation flag). +2. The framework adds **fixed review instructions** (for example: read files from the materials table as needed for the criteria; evidence in the JSON response must be traceable to specific materials). +3. Author `criteria` **still describe what to grade**—authors generally **do not** need to say "read transcript.json" or attachment paths. Configured `attachments` also appear in the materials table automatically. +4. The judge agent **decides which files to open**, like the main agent using Read tools. + +From the author's perspective: write criteria → judge grades. The only change is under the hood—materials are no longer stuffed into argv; the framework provides a "menu" for the judge to consume on demand. + +### Schema Shape + +Add optional `context` to `JudgeConfig`. Case-level `judge` may override eval-level `context` using existing `MergeJudgeConfig` precedence. + +```yaml +judge: + type: agent_judge + context: + profile: standard # minimal | standard + final_message: include # include | omit | truncate | file_ref + transcript: file_ref # include | omit | truncate | file_ref + workspace_diff: file_ref # include | omit | truncate | file_ref + generated_files: index # omit | index | include + limits: + max_bytes: 65536 + transcript_max_turns: 20 + workspace_diff_max_lines: 500 + attachments: + - path: relative/or/absolute/path + label: optional_label +``` + +Profile defaults when fields are omitted: + +| Profile | transcript | workspace_diff | final_message | +| --- | --- | --- | --- | +| `minimal` | `omit` | `omit` | `truncate` | +| `standard` | `file_ref` | `file_ref` | `include` | + +When `judge.context` is entirely omitted, the evaluator applies `profile: standard` (behavior change from today's implicit full inline). + +### Prompt Delivery (All Agents) + +Introduce `internal/agent/prompt_delivery.go`: + +| Mode | When | Command shape | +| --- | --- | --- | +| `inline` | `len(instruction) <= threshold` | Current `claude ... 'instruction'` | +| `file` | Above threshold (default fallback) | Write `$ARTIFACT_DIR/prompt.txt`, invoke CLI with path or wrapper | +| `stdin` | Engine supports reading `-p` from stdin | Pipe instruction to process | + +Constants: + +- Default `SKILL_UP_PROMPT_INLINE_MAX_BYTES = 32768` +- Overridable via environment variable + +All built-in engines (`claude_code`, `codex`, `qodercli`, `qwen_code`) route `Run()` instructions through `deliverPrompt(ctx, rt, opts, instruction)`. + +### Context Materialization (`agent_judge`) + +Introduce `internal/judge/context_materializer.go`: + +Output directory: + +```text +{outputDir}/{caseId}/{variant}/judge/context/ + manifest.json + transcript.json + workspace.diff + final_message.txt + attachments/ +``` + +`buildJudgePrompt` receives a `MaterializedContext` and emits: + +1. **Criteria** (author-written, unchanged) +2. **Materials table** (framework-generated: key, path, size, mode, truncated flag) +3. **Review instructions** (framework template: read files from the materials table as needed for criteria; evidence must be traceable) +4. **Required JSON response schema** + +No multi-megabyte JSON blobs in the prompt string; authors do not repeat material paths in criteria. + +### Runtime Behavior + +```mermaid +sequenceDiagram + participant Ev as Evaluator + participant CM as ContextMaterializer + participant AJ as AgentJudge + participant PD as PromptDelivery + participant Eng as Agent Engine + + Ev->>CM: SessionResult + JudgeContextConfig + CM->>CM: Write judge/context/* + CM-->>AJ: MaterializedContext + AJ->>AJ: buildJudgePrompt (short) + AJ->>PD: deliverPrompt(prompt) + PD->>Eng: inline | file | stdin + Eng-->>AJ: judge JSON output +``` + +Merge semantics: + +- Eval-level `judge.context` is the base. +- Case-level `judge.context` overrides eval-level fields when present (same pattern as other judge fields). +- Unset fields inherit from the resolved profile. + +### Notes/Constraints/Caveats + +1. **Judge agent must be able to read files** in the workspace or artifact directory. For `environment.type: none`, paths must be absolute or workspace-relative and readable by the engine process. +2. **`include` is not unbounded**: framework may auto-downgrade to `file_ref` when `limits.max_bytes` is exceeded. +3. **`attachments.path`** resolves relative to the skill directory, consistent with `script_path` and MCP `config_ref`. +4. **Behavior change**: omitting `judge.context` no longer means "inline everything"; it means `profile: standard`. +5. **Main agent benefits immediately** from `PromptDelivery` even before authors tune `judge.context`. + +### Risks and Mitigations + +| Risk | Mitigation | +| --- | --- | +| Judge LLM ignores the materials table and guesses | Framework injects materials table and read-file instructions in every judge prompt; e2e verifies traceable evidence | +| `standard` profile breaks evals expecting inline transcript | Document migration; use `standard` + `transcript: include` for legacy behavior within limits | +| File mode unsupported by a future engine | `PromptDelivery` falls back per engine capability matrix | +| Disk usage in CI | Context dir lives under existing output artifacts; optional cleanup on `rt.Close()` | +| Attachment path escapes workspace | Validate paths; reject `..` traversal outside skill dir and workspace | + +## Design Details + +### Schema Changes + +```go +type JudgeConfig struct { + // existing fields... + Context *JudgeContextConfig `yaml:"context,omitempty"` +} + +type JudgeContextConfig struct { + Profile string `yaml:"profile,omitempty"` + FinalMessage string `yaml:"final_message,omitempty"` + Transcript string `yaml:"transcript,omitempty"` + WorkspaceDiff string `yaml:"workspace_diff,omitempty"` + GeneratedFiles string `yaml:"generated_files,omitempty"` + Limits *JudgeContextLimits `yaml:"limits,omitempty"` + Attachments []JudgeContextAttachment `yaml:"attachments,omitempty"` +} + +type JudgeContextLimits struct { + MaxBytes int `yaml:"max_bytes,omitempty"` + TranscriptMaxTurns int `yaml:"transcript_max_turns,omitempty"` + WorkspaceDiffMaxLines int `yaml:"workspace_diff_max_lines,omitempty"` +} + +type JudgeContextAttachment struct { + Path string `yaml:"path"` + Label string `yaml:"label,omitempty"` +} +``` + +Validator rules: + +1. `profile` must be one of `minimal` or `standard` when set. +2. Field modes must be one of `include`, `omit`, `truncate`, `file_ref`. +3. `attachments[].path` must be non-empty. +4. `limits.max_bytes` must be non-negative. + +### PromptDelivery + +Location: `internal/agent/prompt_delivery.go` + +```go +func deliverPrompt(ctx context.Context, rt Runtime, opts ExecOptions, instruction string) (command string, err error) +``` + +Responsibilities: + +1. Compare `len(instruction)` with `inlineMaxBytes()`. +2. When using file mode, write to `filepath.Join(opts.ArtifactDir, "prompt.txt")`. +3. Return a short shell command that does not embed the full instruction. +4. Record delivery mode in context for observability. + +### ContextMaterializer + +Location: `internal/judge/context_materializer.go` + +```go +type MaterializedContext struct { + Dir string + Manifest ContextManifest + Materials []ContextMaterial +} + +func MaterializeJudgeContext( + ctx context.Context, + rt runtime.Runtime, + cfg *config.JudgeContextConfig, + in Input, + artifactDir string, +) (*MaterializedContext, error) +``` + +Responsibilities: + +1. Resolve effective config from profile + explicit overrides. +2. Write selected artifacts under `judge/context/`. +3. Copy `attachments` into `judge/context/attachments/`. +4. Build `manifest.json` with per-field byte counts and truncation flags. + +### AgentJudge Changes + +`AgentJudge.Evaluate`: + +1. Call `MaterializeJudgeContext` before `buildJudgePrompt`. +2. Pass `MaterializedContext` into `buildJudgePrompt`. +3. Set `judgeInput.ArtifactDir` (already done by evaluator) so `PromptDelivery` can write `prompt.txt` beside context files. + +`buildJudgePrompt` stops unconditionally marshaling the full transcript into the prompt string. + +### Evaluator Changes + +1. `runJudgePhaseWithSpan`: no change to judge selection. +2. `newJudgeForCase`: pass resolved `JudgeContextConfig` into `NewAgentJudge` or let `AgentJudge` read it from config. +3. `prepareWorkspaceArtifacts`: still collects workspace diff into `SessionResult`, but materializer decides whether diff enters the prompt. +4. `grading.json`: attach `judge_context` from materializer manifest and prompt delivery metadata. + +### Observability + +Add to judge grading metadata: + +```json +{ + "judge_context": { + "profile": "minimal", + "materialized_dir": ".../judge/context", + "manifest": { + "transcript": { "mode": "omit", "bytes": 0 }, + "workspace_diff": { "mode": "omit", "bytes": 0 }, + "final_message": { "mode": "truncate", "bytes": 4096, "original_bytes": 12000 } + }, + "prompt_delivery": "file", + "prompt_bytes": 2048 + } +} +``` + +Log lines: + +```text +level=INFO msg="judge context materialized" profile=minimal dir=... +level=INFO msg="prompt delivery" mode=file bytes=2048 threshold=32768 +``` + +### Documentation and Templates + +Update: + +1. `docs/guide/writing-evals.md` — `judge.context` and profiles. +2. `docs/zh/guide/writing-evals.md` — Chinese mirror. +3. `skills/skill-upper/assets/eval.yaml.tmpl` — mention `agent_judge` context profiles. +4. `CHANGELOG.md` — note default behavior change for `agent_judge`. +5. Product workspace doc may link to `proposals/0004-agent-judge-context-delivery.md`. + +## Test Plan + +### Unit Tests + +1. `internal/agent/prompt_delivery_test.go` + - 31KB inline, 33KB file mode; + - resulting argv length below safe bound; + - writes `prompt.txt` under `ArtifactDir`. + +2. `internal/judge/context_materializer_test.go` + - profile resolution for `minimal` / `standard`; + - `omit`, `file_ref`, `truncate`, auto-downgrade from `include`; + - attachment copy and manifest generation. + +3. `internal/judge/agent_judge_test.go` + - `buildJudgePrompt` with materialized context does not contain full transcript JSON; + - evaluate path records manifest metadata. + +4. `internal/config/validator_test.go` + - invalid profile/mode rejected; + - valid `judge.context` loads from YAML. + +5. `internal/evaluator/evaluator_test.go` + - synthetic 2MB transcript: judge phase does not build multi-MB argv; + - `judge_context` appears in grading output. + +### Integration/E2E Tests + +Add fixture: + +```text +e2e/testdata/agent-judge-large-context/ + SKILL.md + evals/eval.yaml + evals/cases/large-transcript.yaml +``` + +Case uses a mock/custom engine that returns a large transcript. Assert: + +1. Judge phase completes without `argument list too long`. +2. `judge/context/transcript.json` exists. +3. Judge prompt file or inline prompt is below threshold. + +### Manual Verification + +```bash +make fmt +make verify +make test +go test -tags e2e -v ./e2e -run TestAgentJudge_LargeContext +``` + +Re-run a previously failing long-running CI case with `context.profile: minimal`. + +## Drawbacks + +1. Default `standard` profile changes behavior for existing `agent_judge` evals that implicitly relied on inline transcript (author criteria style can stay the same). +2. Implementation must keep the materials table and review instructions clear enough for judges to read files on demand without criteria changes. +3. Additional disk I/O for context materialization (usually negligible vs agent runtime). +4. Engine-specific file prompt support may require per-engine tuning in `PromptDelivery`. + +## Alternatives + +### Alternative A: `include_transcript: false` only + +Minimal change, but does not address `workspace_diff`, main-agent argv limits, or attachment extensibility. + +### Alternative B: Switch the affected skill to `script_judge` + +Works for that skill, but removes LLM semantic review for branches that need it and diverges from the documented companion judge skill + `agent_judge` product shape. + +### Alternative C: Hard cap transcript in code with no config + +Reduces failures but hides truncation from authors and breaks audit scenarios that need full context visibility. + +### Alternative D: Judge via direct model API (no CLI) + +Avoids argv limits entirely but breaks skill-up's "real engine" positioning and duplicates engine auth paths. + +## Infrastructure Needed + +No new external services. + +Implementation requires Go changes in `internal/agent`, `internal/judge`, `internal/evaluator`, `internal/config`, tests, and documentation. CI runners need no change beyond consuming smaller judge argv. + +## Upgrade & Migration Strategy + +Phased rollout: + +### Phase 1 (P0) + +- `PromptDelivery` for `claude_code` +- `judge.context` with profiles `minimal` and `standard` +- `judge_context` report metadata +- long-running benchmark evals adopt `profile: minimal` as the reference configuration + +### Phase 2 (P1) + +- `attachments`, fine-grained limits, engine parity +- writing guides and skill-upper template updates +- e2e coverage for file-based judging and `limits` overrides under `standard` + +### Phase 3 (P2) + +- shared materialize package with `script_judge` +- `skill-up validate` recommendations + +Backward compatibility: + +1. Short prompts continue to use inline delivery. +2. Evals needing legacy inline transcript set `context.transcript: include` (subject to `limits.max_bytes`). +3. Document the default shift from implicit inline to `standard` file-reference behavior in CHANGELOG. + +Documentation should state: + +- SUP-0004 introduces `judge.context` and `PromptDelivery`; +- `agent_judge` defaults to materialized context, not argv-inlined megabyte prompts; +- long-running benchmark evals with `attachments` or script/file-only grading should prefer `profile: minimal`. diff --git a/proposals/README.md b/proposals/README.md index 47a280e..cefd5ae 100644 --- a/proposals/README.md +++ b/proposals/README.md @@ -9,3 +9,4 @@ This is the complete list of skill-up Enhancement Proposals: | [SUP-0001](0001-multi-turn-conversation-eval.md) | Multi-Turn Conversation Evaluation Support | [中文](zh/0001-multi-turn-conversation-eval.md) | provisional | 2026-05-19 | | [SUP-0002](0002-agent-judge-specific-skill.md) | agent_judge Judge-Specific Skill Support | [中文](zh/0002-agent-judge-specific-skill.md) | draft | 2026-07-07 | | [SUP-0003](0003-per-case-mocked-mcp-responses.md) | Per-Case Mocked MCP Response Overrides | [中文](zh/0003-per-case-mocked-mcp-responses.md) | draft | 2026-07-07 | +| [SUP-0004](0004-agent-judge-context-delivery.md) | Agent Judge Context Delivery and Scale Control | [中文](zh/0004-agent-judge-context-delivery.md) | draft | 2026-07-07 | diff --git a/proposals/zh/0004-agent-judge-context-delivery.md b/proposals/zh/0004-agent-judge-context-delivery.md new file mode 100644 index 0000000..fcc657a --- /dev/null +++ b/proposals/zh/0004-agent-judge-context-delivery.md @@ -0,0 +1,589 @@ +--- +title: agent_judge 上下文传递与规模控制 +authors: + - "kongtang" +creation-date: 2026-07-07 +last-updated: 2026-07-07 +status: draft +--- + +# SUP-0004: agent_judge 上下文传递与规模控制 + +语言:[English](../0004-agent-judge-context-delivery.md) | 中文 + + +- [SUP-0004: agent\_judge 上下文传递与规模控制](#sup-0004-agent_judge-上下文传递与规模控制) + - [摘要](#摘要) + - [动机](#动机) + - [目标](#目标) + - [非目标](#非目标) + - [需求](#需求) + - [必须有](#必须有) + - [应该有](#应该有) + - [最好有](#最好有) + - [提案](#提案) + - [用户场景速查](#用户场景速查) + - [场景 1:长时仓库变更 Benchmark(`profile: minimal`)](#场景-1长时仓库变更-benchmarkprofile-minimal) + - [场景 2:代码审查 Skill(`profile: standard`,默认)](#场景-2代码审查-skillprofile-standard默认) + - [分字段模式说明](#分字段模式说明) + - [作者体验:材料传递对用户无感](#作者体验材料传递对用户无感) + - [Schema 形态](#schema-形态) + - [Prompt 传递(所有 Agent)](#prompt-传递所有-agent) + - [上下文物化(agent\_judge)](#上下文物化agent_judge) + - [运行时行为](#运行时行为) + - [注意事项/约束/说明](#注意事项约束说明) + - [风险与缓解措施](#风险与缓解措施) + - [设计细节](#设计细节) + - [Schema 变更](#schema-变更) + - [PromptDelivery](#promptdelivery) + - [ContextMaterializer](#contextmaterializer) + - [AgentJudge 改造](#agentjudge-改造) + - [Evaluator 改造](#evaluator-改造) + - [可观测性](#可观测性) + - [文档与模板](#文档与模板) + - [测试计划](#测试计划) + - [单元测试](#单元测试) + - [集成/E2E 测试](#集成e2e-测试) + - [手工验证](#手工验证) + - [缺点](#缺点) + - [替代方案](#替代方案) + - [方案 A:仅增加 `include_transcript: false`](#方案-a仅增加-include_transcript-false) + - [方案 B:将受影响 Skill 整体改为 `script_judge`](#方案-b将受影响-skill-整体改为-script_judge) + - [方案 C:代码硬截断 transcript、无配置](#方案-c代码硬截断-transcript无配置) + - [方案 D:Judge 直连模型 API(不经 CLI)](#方案-djudge-直连模型-api不经-cli) + - [所需基础设施](#所需基础设施) + - [升级与迁移策略](#升级与迁移策略) + - [Phase 1(P0)](#phase-1p0) + - [Phase 2(P1)](#phase-2p1) + - [Phase 3(P2)](#phase-3p2) + + +## 摘要 + +当前 skill-up 的 `agent_judge` 会将完整的 agent `transcript`、`final_message` 和 `workspace_diff` 内联拼进 judge prompt,再通过 `bash -c 'claude -p "<整段 prompt>"'` 传给 Claude Code 等 Agent Engine。长时任务、多工具调用和大体积 workspace diff 容易超过 OS `ARG_MAX` 上限,导致 `fork/exec /usr/bin/bash: argument list too long`。 + +本提案引入两层互补能力: + +1. **Prompt 传递(所有 Agent)**:超大 prompt 自动改用文件或 stdin 传递,不再嵌入 argv。 +2. **`judge.context`(仅 agent_judge)**:评测作者通过 `minimal` / `standard` profile 声明需要物化、引用、省略或截断哪些评审材料。 + +该设计与现有 `script_judge` 的「按路径传 artifact」思路对齐,同时保留 criteria + evidence JSON 的 LLM 评分形态。 + +## 动机 + +某长时 Skill 评测的生产 CI(某一高负载 case)在主 agent 执行完成后、judge 阶段失败: + +```text +claude-code run failed: fork/exec /usr/bin/bash: argument list too long +agent_judge failed to parse agent output: no valid JSON found in agent output +``` + +根因分两层: + +| 层级 | 机制 | 影响 | +| ---- | ------------------------------------------------------- | --------------------------------------------------- | +| 传递 | `internal/agent/claude_code.go` → `buildClaudePrintCmd` | 整段 instruction 作为 shell 参数传入 | +| 内容 | `internal/judge/agent_judge.go` → `buildJudgePrompt` | 内联 transcript JSON、workspace diff、final message | + +长 case(`timeout_seconds: 3600`、`max_turns: 50`)下,仅 transcript 即可达数 MB。`agent_judge` 还会默认收集 workspace git diff(`judgeNeedsWorkspaceDiff` 对所有 `agent_judge` 返回 true),在 CI 检出完整应用仓库时体积同样可观。 + +仅增加 `include_transcript: false` 无法解决: + +1. 与 transcript 无关的大体积 `workspace_diff`。 +2. 主 agent case prompt 走同一 argv 传递路径的问题。 +3. 未来共性场景:多轮评测、代码审查类 Skill、仓库级变更类 benchmark。 +4. 上下文被截断或降级时缺乏可观测性。 + +### 目标 + +1. **消除 ARG_MAX 类失败**:agent 与 judge 在常见大规模评测负载下稳定启动。 +2. **可配置 judge 上下文**:通过 `judge.context`(profile / attachments)控制材料范围;**criteria 写法保持不变**。 +3. **默认安全**:新 eval 不再静默将 MB 级上下文内联进 argv。 +4. **与 script_judge 对齐**:大材料落盘,prompt 只引用路径。 +5. **可观测**:报告记录物化模式、体积、截断与 prompt 传递方式。 +6. **兼容路径**:现有短 prompt eval 保持 inline 行为。 + +### 非目标 + +1. **不改变 `rule_based` 语义**:内存中的 transcript 断言逻辑不变。 +2. **不将 `agent_judge` 整体替换为 `script_judge`**:保留 criteria + evidence 的 LLM 评分。 +3. **MVP 不引入直连 HTTP/API judge**:仍通过现有 CLI adapter 运行 engine。 +4. **MVP 不做 criteria 自动推断 profile**:profile 由作者显式声明;智能推荐后续迭代。 +5. **不跨 case 共享 judge 上下文文件**:每个 case variant 使用独立 `judge/context/` 目录。 + +## 需求 + +### 必须有 + +| ID | 需求 | 验收标准 | +| --- | ---------------------- | ----------------------------------------------------------------------------------- | +| R1 | 大 prompt 传递 | 超阈值 prompt 使用 file/stdin;多 MB judge prompt 不再触发 `argument list too long` | +| R2 | `judge.context` schema | `JudgeConfig` 支持 `context` 及 profile、分字段模式 | +| R3 | 上下文物化 | `agent_judge` 按配置将材料写入 `judge/context/` | +| R4 | 短 judge prompt | 默认 `standard` profile 下 judge prompt 体积有界(路径引用,非内联 JSON) | +| R5 | 自动降级 | `include` 模式受 `limits.max_bytes` 约束,超限自动降为 `file_ref` | +| R6 | 报告元数据 | `grading.json` / `result.json` 包含 `judge_context` manifest | +| R7 | 向后兼容 | 无 `judge.context` 的短 prompt eval 通过现有测试 | + +### 应该有 + +| ID | 需求 | 验收标准 | +| --- | ------------------ | ------------------------------------------------------ | +| S1 | Profile 预设 | `minimal`、`standard` 语义清晰且有文档 | +| S2 | `attachments` 支持 | 作者可按路径附加业务产物(如 `diff-result.json`) | +| S3 | 多 Engine 一致 | `codex`、`qodercli`、`qwen_code` 共用 `PromptDelivery` | +| S4 | 写作指南更新 | 中英文 eval 写作指南包含 `judge.context` 示例 | + +### 最好有 + +| ID | 需求 | 验收标准 | +| --- | ------------------------ | ----------------------------------------------------------------------------------- | +| N1 | ARG_MAX 探测 | 可选的 exec 前体积检查与强制 file 模式日志 | +| N2 | `skill-up validate` 提示 | 已配置 `attachments` 但 profile 仍将大段 transcript 内联时,建议 `profile: minimal` | +| N3 | 共享物化包 | `script_judge` 与 `agent_judge` 复用同一 artifact 写入逻辑 | + +## 提案 + +### 用户场景速查 + +#### 场景 1:长时仓库变更 Benchmark(`profile: minimal`) + +评测主要依据脚本产出与 diff 文件,而非对话历史: + +```yaml +judge: + type: agent_judge + model: anthropic/claude-sonnet-4-6 + context: + profile: minimal + attachments: + - path: evals/fixtures/artifacts/diff-result.json + label: diff_result + criteria: + - "判定代码变更是否符合预期规则。" + - "过滤临时目录后若结果一致,须引用可核验证据并通过。" + pass_threshold: 1 +``` + +要点: + +- transcript 与 workspace diff 不进入 judge prompt。 +- 业务证据通过 `attachments` 物化;**路径由框架自动写入 judge 材料表**,作者无需在 criteria 中声明文件路径。 +- Judge prompt 保持短小;judge agent 根据 criteria 自行决定是否读取附件。 + +#### 场景 2:代码审查 Skill(`profile: standard`,默认) + +```yaml +judge: + type: agent_judge + model: anthropic/claude-sonnet-4-6 + context: + profile: standard + criteria: + - "识别真实 bug 并给出准确位置" + - "未将正确代码误报为 bug" +``` + +生效行为: + +- `transcript.json`、`workspace.diff`、`final_message.txt` 写入 `judge/context/`。 +- 框架在 judge prompt 中**自动注入材料表**(路径、大小、模式);作者 criteria **写法与今天相同**,无需提及文件名。 +- judge agent 根据 criteria 自行打开需要的文件撰写 evidence。 +- 需要更宽松截断或保留策略时,在 `standard` 下通过 `limits` 与分字段覆盖调整(无需额外 profile)。 + +### 分字段模式说明 + +`final_message`、`transcript`、`workspace_diff` 等字段均可单独指定投递方式(覆盖 profile 默认值): + +| 模式 | judge prompt 中 | 磁盘 artifact | 作用 | +| ---------- | ------------------------- | ------------- | ------------------------------------------------------------------------- | +| `include` | 内联原文 | 可选镜像 | 材料较短,希望 judge 直接看到全文 | +| `file_ref` | 仅路径/引用,不含全文 | 必写完整文件 | 材料较大,避免撑爆 prompt / argv(`standard` 对 transcript、diff 的默认) | +| `omit` | 不出现 | 不写 | 评审不需要该类材料(`minimal` 对 transcript、diff 的默认) | +| `truncate` | 内联摘要 +「全文见 path」 | 写完整版 | prompt 里保留预览,全文在磁盘(`minimal` 对 `final_message` 的默认) | + +**兜底**:即使配置 `include`,单段超过 `limits.max_bytes` 时框架也会自动降为 `file_ref`。 + +`generated_files` 使用相近语义:`omit`(不要)、`index`(仅路径列表)、`include`(内联内容)。 + +### 作者体验:材料传递对用户无感 + +此前 `agent_judge` 将 transcript、diff 等**内联进 judge prompt**;作者只写 `criteria`,不必关心材料如何送达 judge。 + +本方案**只改传递机制,不改作者心智模型**: + +1. 框架按 `profile` / 分字段配置物化材料,并在 judge 的 system prompt 中**自动注入材料表**(字段名、路径、大小、是否截断)。 +2. 框架附带**固定评审指引**(例如:根据 criteria 按需读取材料表中的文件;JSON 回应中的 evidence 须可溯源到具体材料)。 +3. 作者的 `criteria` **仍只描述「评什么」**,一般**不必**写「请读取 transcript.json」或 attachment 路径——`attachments` 配置后也会自动出现在材料表中。 +4. judge agent 像主 agent 使用 Read 工具一样,**自行判断**需要打开哪些文件。 + +对作者而言:继续写 criteria → judge 评审;差异仅在实现层——材料不再塞进命令行,而由框架写好「菜单」供 judge 按需取用。 + +### Schema 形态 + +在 `JudgeConfig` 增加可选 `context`。case 级 `judge.context` 按现有 `MergeJudgeConfig` 优先级覆盖 eval 级配置。 + +```yaml +judge: + type: agent_judge + context: + profile: standard # minimal | standard + final_message: include # include | omit | truncate | file_ref + transcript: file_ref # include | omit | truncate | file_ref + workspace_diff: file_ref # include | omit | truncate | file_ref + generated_files: index # omit | index | include + limits: + max_bytes: 65536 + transcript_max_turns: 20 + workspace_diff_max_lines: 500 + attachments: + - path: relative/or/absolute/path + label: optional_label +``` + +省略分字段时的 profile 默认: + +| Profile | transcript | workspace_diff | final_message | +| ---------- | ---------- | -------------- | ------------- | +| `minimal` | `omit` | `omit` | `truncate` | +| `standard` | `file_ref` | `file_ref` | `include` | + +完全省略 `judge.context` 时,evaluator 应用 `profile: standard`(相对今天隐式全量内联,属于行为变更)。 + +### Prompt 传递(所有 Agent) + +新增 `internal/agent/prompt_delivery.go`: + +| 模式 | 条件 | 命令形态 | +| -------- | ------------------------------- | ----------------------------------------------- | +| `inline` | `len(instruction) <= threshold` | 现有 `claude ... 'instruction'` | +| `file` | 超过阈值(默认回落) | 写入 `$ARTIFACT_DIR/prompt.txt`,短命令引用路径 | +| `stdin` | Engine 支持从 stdin 读 `-p` | 管道传入 instruction | + +常量: + +- 默认 `SKILL_UP_PROMPT_INLINE_MAX_BYTES = 32768` +- 可通过环境变量覆盖 + +所有内置 engine(`claude_code`、`codex`、`qodercli`、`qwen_code`)的 `Run()` 统一经 `deliverPrompt(ctx, rt, opts, instruction)` 构造命令。 + +### 上下文物化(agent_judge) + +新增 `internal/judge/context_materializer.go`: + +输出目录: + +```text +{outputDir}/{caseId}/{variant}/judge/context/ + manifest.json + transcript.json + workspace.diff + final_message.txt + attachments/ +``` + +`buildJudgePrompt` 接收 `MaterializedContext`,输出: + +1. **Criteria**(作者编写,不变) +2. **材料表**(框架自动生成:key、path、size、mode、是否截断) +3. **评审指引**(框架固定模板:根据 criteria 按需读取材料表中的文件;evidence 须可溯源) +4. **要求的 JSON 响应格式** + +prompt 字符串中不再包含数 MB 的 JSON 块;作者无需在 criteria 中重复材料路径。 + +### 运行时行为 + +```mermaid +sequenceDiagram + participant Ev as Evaluator + participant CM as ContextMaterializer + participant AJ as AgentJudge + participant PD as PromptDelivery + participant Eng as Agent Engine + + Ev->>CM: SessionResult + JudgeContextConfig + CM->>CM: 写入 judge/context/* + CM-->>AJ: MaterializedContext + AJ->>AJ: buildJudgePrompt(短 prompt) + AJ->>PD: deliverPrompt(prompt) + PD->>Eng: inline | file | stdin + Eng-->>AJ: judge JSON 输出 +``` + +合并语义: + +- eval 级 `judge.context` 为基础配置。 +- case 级 `judge.context` 覆盖同名字段(与其他 judge 字段一致)。 +- 未显式设置的字段继承 resolved profile。 + +### 注意事项/约束/说明 + +1. **Judge agent 必须能读文件**:`environment.type: none` 时,路径须对 engine 进程可读(绝对路径或 workspace 相对路径)。 +2. **`include` 非无界**:超 `limits.max_bytes` 时框架自动降为 `file_ref`。 +3. **`attachments.path`** 相对 Skill 目录解析,与 `script_path`、MCP `config_ref` 一致。 +4. **行为变更**:省略 `judge.context` 不再表示「全部内联」,而表示 `profile: standard`。 +5. **主 agent 同步受益**:即使作者未配置 `judge.context`,`PromptDelivery` 也会保护 case agent。 + +### 风险与缓解措施 + +| 风险 | 缓解 | +| -------------------------------------------- | ------------------------------------------------------------------------ | +| Judge LLM 忽略材料表、凭空打分 | 框架在 judge prompt 固定注入材料表与读文件指引;e2e 验证 evidence 可溯源 | +| `standard` 破坏依赖内联 transcript 的旧 eval | 文档说明迁移路径;`standard` + `transcript: include` 保留有限内联 | +| 未来 engine 不支持 file 模式 | `PromptDelivery` 按 engine 能力矩阵回落 | +| CI 磁盘占用 | 上下文目录属于现有 artifact;可在 `rt.Close()` 时清理 | +| attachment 路径穿越 | 校验路径,拒绝逃出 skill 目录与 workspace 的 `..` | + +## 设计细节 + +### Schema 变更 + +```go +type JudgeConfig struct { + // 现有字段... + Context *JudgeContextConfig `yaml:"context,omitempty"` +} + +type JudgeContextConfig struct { + Profile string `yaml:"profile,omitempty"` + FinalMessage string `yaml:"final_message,omitempty"` + Transcript string `yaml:"transcript,omitempty"` + WorkspaceDiff string `yaml:"workspace_diff,omitempty"` + GeneratedFiles string `yaml:"generated_files,omitempty"` + Limits *JudgeContextLimits `yaml:"limits,omitempty"` + Attachments []JudgeContextAttachment `yaml:"attachments,omitempty"` +} + +type JudgeContextLimits struct { + MaxBytes int `yaml:"max_bytes,omitempty"` + TranscriptMaxTurns int `yaml:"transcript_max_turns,omitempty"` + WorkspaceDiffMaxLines int `yaml:"workspace_diff_max_lines,omitempty"` +} + +type JudgeContextAttachment struct { + Path string `yaml:"path"` + Label string `yaml:"label,omitempty"` +} +``` + +校验规则: + +1. `profile` 若设置,必须为 `minimal`、`standard` 之一。 +2. 分字段模式必须为 `include`、`omit`、`truncate`、`file_ref` 之一。 +3. `attachments[].path` 非空。 +4. `limits.max_bytes` 非负。 + +### PromptDelivery + +位置:`internal/agent/prompt_delivery.go` + +```go +func deliverPrompt(ctx context.Context, rt Runtime, opts ExecOptions, instruction string) (command string, err error) +``` + +职责: + +1. 比较 `len(instruction)` 与 `inlineMaxBytes()`。 +2. file 模式下写入 `filepath.Join(opts.ArtifactDir, "prompt.txt")`。 +3. 返回不嵌入完整 instruction 的短 shell 命令。 +4. 将 delivery 模式写入 context 供可观测性使用。 + +### ContextMaterializer + +位置:`internal/judge/context_materializer.go` + +```go +type MaterializedContext struct { + Dir string + Manifest ContextManifest + Materials []ContextMaterial +} + +func MaterializeJudgeContext( + ctx context.Context, + rt runtime.Runtime, + cfg *config.JudgeContextConfig, + in Input, + artifactDir string, +) (*MaterializedContext, error) +``` + +职责: + +1. 由 profile + 显式覆盖解析最终配置。 +2. 将选中材料写入 `judge/context/`。 +3. 将 `attachments` 复制到 `judge/context/attachments/`。 +4. 生成含各字段字节数与截断标记的 `manifest.json`。 + +### AgentJudge 改造 + +`AgentJudge.Evaluate`: + +1. 在 `buildJudgePrompt` 前调用 `MaterializeJudgeContext`。 +2. 将 `MaterializedContext` 传入 `buildJudgePrompt`。 +3. 利用 evaluator 已设置的 `judgeInput.ArtifactDir`,使 `PromptDelivery` 将 `prompt.txt` 写在 context 旁。 + +`buildJudgePrompt` 不再无条件将完整 transcript marshal 进 prompt 字符串。 + +### Evaluator 改造 + +1. `runJudgePhaseWithSpan`:judge 选择逻辑不变。 +2. `newJudgeForCase`:将 resolved `JudgeContextConfig` 传入 `NewAgentJudge`,或由 `AgentJudge` 从 config 读取。 +3. `prepareWorkspaceArtifacts`:仍将 workspace diff 写入 `SessionResult`;是否进入 prompt 由 materializer 决定。 +4. `grading.json`:附加 materializer manifest 与 prompt delivery 元数据。 + +### 可观测性 + +在 judge grading 元数据中增加: + +```json +{ + "judge_context": { + "profile": "minimal", + "materialized_dir": ".../judge/context", + "manifest": { + "transcript": { "mode": "omit", "bytes": 0 }, + "workspace_diff": { "mode": "omit", "bytes": 0 }, + "final_message": { "mode": "truncate", "bytes": 4096, "original_bytes": 12000 } + }, + "prompt_delivery": "file", + "prompt_bytes": 2048 + } +} +``` + +日志示例: + +```text +level=INFO msg="judge context materialized" profile=minimal dir=... +level=INFO msg="prompt delivery" mode=file bytes=2048 threshold=32768 +``` + +### 文档与模板 + +更新: + +1. `docs/guide/writing-evals.md` — `judge.context` 与 profile。 +2. `docs/zh/guide/writing-evals.md` — 中文镜像。 +3. `skills/skill-upper/assets/eval.yaml.tmpl` — 补充 `agent_judge` context profile 说明。 +4. `CHANGELOG.md` — 记录 `agent_judge` 默认行为变更。 +5. 产品工作区设计文档可链接至本提案。 + +## 测试计划 + +### 单元测试 + +1. `internal/agent/prompt_delivery_test.go` + - 31KB inline、33KB file 模式; + - 结果 argv 长度低于安全边界; + - 在 `ArtifactDir` 下生成 `prompt.txt`。 + +2. `internal/judge/context_materializer_test.go` + - `minimal` / `standard` profile 解析; + - `omit`、`file_ref`、`truncate`、`include` 自动降级; + - attachment 复制与 manifest 生成。 + +3. `internal/judge/agent_judge_test.go` + - 物化上下文的 `buildJudgePrompt` 不含完整 transcript JSON; + - evaluate 路径写入 manifest 元数据。 + +4. `internal/config/validator_test.go` + - 非法 profile/mode 被拒绝; + - 合法 `judge.context` 可从 YAML 加载。 + +5. `internal/evaluator/evaluator_test.go` + - 合成 2MB transcript:judge 阶段不构造多 MB argv; + - grading 输出含 `judge_context`。 + +### 集成/E2E 测试 + +新增 fixture: + +```text +e2e/testdata/agent-judge-large-context/ + SKILL.md + evals/eval.yaml + evals/cases/large-transcript.yaml +``` + +使用返回大 transcript 的 mock/custom engine。断言: + +1. Judge 阶段不出现 `argument list too long`。 +2. 存在 `judge/context/transcript.json`。 +3. Judge prompt 文件或 inline prompt 低于阈值。 + +### 手工验证 + +```bash +make fmt +make verify +make test +go test -tags e2e -v ./e2e -run TestAgentJudge_LargeContext +``` + +使用 `context.profile: minimal` 重跑此前因 judge prompt 过大而失败的长时 CI case。 + +## 缺点 + +1. 默认 `standard` profile 会改变依赖隐式内联 transcript 的现有 `agent_judge` eval 行为(作者 criteria 写法可不变)。 +2. 实现层须保证材料表与评审指引足够清晰,使 judge 在不改 criteria 的前提下仍能按需读文件。 +3. 上下文物化带来额外磁盘 I/O(相对 agent 运行时间通常可忽略)。 +4. 各 engine 的 file prompt 支持可能需在 `PromptDelivery` 中分别适配。 + +## 替代方案 + +### 方案 A:仅增加 `include_transcript: false` + +改动最小,但不解决 `workspace_diff`、主 agent argv 限制与 attachment 扩展性。 + +### 方案 B:将受影响 Skill 整体改为 `script_judge` + +对该 Skill 有效,但失去需要 LLM 语义评审的分支能力,且偏离「配套 judge Skill + `agent_judge`」的产品形态。 + +### 方案 C:代码硬截断 transcript、无配置 + +可降低失败率,但作者无法感知截断,且损害审计场景。 + +### 方案 D:Judge 直连模型 API(不经 CLI) + +可避开 argv 限制,但违背 skill-up「真实 engine」定位,并重复鉴权路径。 + +## 所需基础设施 + +无需新增外部服务。 + +实现涉及 `internal/agent`、`internal/judge`、`internal/evaluator`、`internal/config` 的 Go 改动、测试与文档。CI runner 除消费更小的 judge argv 外无需变更。 + +## 升级与迁移策略 + +分阶段发布: + +### Phase 1(P0) + +- `claude_code` 接入 `PromptDelivery` +- `judge.context` 支持 `minimal` 与 `standard` profile +- `judge_context` 报告元数据 +- 长时 benchmark 类 eval 采用 `profile: minimal` 作为参考配置 + +### Phase 2(P1) + +- `attachments`、细粒度 limits、多 engine 一致 +- 写作指南与 skill-upper 模板更新 +- `standard` profile 下读文件与 `limits` 覆盖的 e2e 覆盖 + +### Phase 3(P2) + +- 与 `script_judge` 共享物化包 +- `skill-up validate` 推荐提示 + +向后兼容: + +1. 短 prompt 继续使用 inline 传递。 +2. 需要旧式内联 transcript 的 eval 设置 `context.transcript: include`(受 `limits.max_bytes` 约束)。 +3. 在 CHANGELOG 中说明默认从隐式内联迁移到 `standard` 文件引用行为。 + +文档应明确: + +- SUP-0004 引入 `judge.context` 与 `PromptDelivery`; +- `agent_judge` 默认物化上下文,不再将 MB 级材料内联进 argv; +- 长时 benchmark 在配置了 `attachments` 或仅需脚本/文件产物时,宜使用 `profile: minimal`。 diff --git a/skills/skill-upper/assets/eval.yaml.tmpl b/skills/skill-upper/assets/eval.yaml.tmpl index 6ae749c..24b50f9 100644 --- a/skills/skill-upper/assets/eval.yaml.tmpl +++ b/skills/skill-upper/assets/eval.yaml.tmpl @@ -45,5 +45,13 @@ cases: # benchmark: # enabled: true +# judge: +# type: agent_judge +# model: anthropic/claude-sonnet-4-6 +# context: +# profile: standard # standard = file refs for transcript/diff; minimal = omit them +# criteria: +# - "The response satisfies the case-specific success criteria." + report: formats: [json]