diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 6e4a9add9..68a72a493 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,4 +1,4 @@ -FROM ghcr.io/framework-r-d/phlex-dev:latest +FROM ghcr.io/framework-r-d/phlex-dev:2026-06-24 # Validate Python site-packages symlink. # Invoke bash explicitly: the script uses bash features (pipefail, arrays, @@ -88,15 +88,26 @@ if [ "$(dpkg-divert --truename /usr/bin/man)" = "/usr/bin/man.REAL" ]; then fi INSTALL_MAN_TOOLS -# Install podman client and socat inside the container to support nested +# Install dev utils # container communication and provide a 'docker' command alias. -RUN bash <<'INSTALL_PODMAN_CLIENT' +RUN bash <<'INSTALL_DEV_UTILS' set -euo pipefail apt-get update -apt-get install -y --no-install-recommends ssh podman socat +apt-get install -y --no-install-recommends jq podman ssh socat tree yq +apt purge nodejs npm -y && apt autoremove -y || true +sudo apt-get install -y curl +curl -fsSL https://deb.nodesource.com/setup_26.x | bash - +apt-get install -y nodejs apt-get clean rm -rf /var/lib/apt/lists/* -INSTALL_PODMAN_CLIENT +INSTALL_DEV_UTILS + +RUN bash <<'INSTALL_NPM_PACKAGES' +set -euo pipefail +npm -g update +npm config set allow-scripts=@kilocode/cli --location=user +npm install -g @kilocode/cli +INSTALL_NPM_PACKAGES # Wire up the root user's .bashrc to source the Spack environment via # /entrypoint.sh on every interactive shell. diff --git a/.devcontainer/codespace.code-workspace b/.devcontainer/codespace.code-workspace index c208ed9c2..b7b289bf1 100644 --- a/.devcontainer/codespace.code-workspace +++ b/.devcontainer/codespace.code-workspace @@ -114,7 +114,6 @@ "extensions": { "recommendations": [ "charliermarsh.ruff", - "github.copilot-chat", "github.vscode-github-actions", "github.vscode-pull-request-github", "ms-vscode.cmake-tools", diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index c60f4acb5..91b3276d7 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -1,47 +1,17 @@ { "name": "Phlex CI Dev Container", - "build": { - "dockerfile": "Dockerfile" - }, - "runArgs": [], + "dockerComposeFile": "docker-compose.yml", + "service": "phlex-dev", "workspaceFolder": "/workspaces/phlex", "remoteUser": "root", "remoteEnv": { - // KILO_CONFIG_CONTENT_DOCKER rewrites 127.0.0.1:PORT to - // host.docker.internal:RELAY_PORT for the socat relay used when - // headroom runs as a local proxy. Passed through here so that - // post-create.sh can wire it into the container's .bashrc - // conditionally: if non-empty it sets KILO_CONFIG_CONTENT to this - // value; otherwise KILO_CONFIG_CONTENT is left unset and Kilo falls - // back to ~/.config/kilo/kilo.jsonc (bind-mounted from the host). - "KILO_CONFIG_CONTENT_DOCKER": "${localEnv:KILO_CONFIG_CONTENT_DOCKER}", "KILO_API_KEY": "${localEnv:HEADROOM_UPSTREAM_KEY}" }, - "containerEnv": { - "CMAKE_GENERATOR": "Ninja", - "GH_CONFIG_DIR": "/root/.config/gh", - "DOCKER_HOST": "unix:///tmp/podman.sock", - "CONTAINER_HOST": "unix:///tmp/podman.sock", - "GNUPGHOME": "/root/.gnupg" - }, - "mounts": [ - "source=${localWorkspaceFolder}/../phlex-coding-guidelines,target=/workspaces/phlex-coding-guidelines,type=bind", - "source=${localWorkspaceFolder}/../phlex-design,target=/workspaces/phlex-design,type=bind", - "source=${localWorkspaceFolder}/../phlex-examples,target=/workspaces/phlex-examples,type=bind", - "source=${localWorkspaceFolder}/../phlex-spack-recipes,target=/workspaces/phlex-spack-recipes,type=bind", - "source=${localEnv:HOME}/.aws,target=/root/.aws,type=bind", - "source=${localEnv:HOME}/.config/gh,target=/root/.config/gh,type=bind,readonly", - "source=${localEnv:HOME}/.config/kilo,target=/root/.config/kilo,type=bind", - "source=${localEnv:HOME}/.gnupg,target=/root/.gnupg,type=bind", - "source=${localEnv:HOME}/.kiro,target=/root/.kiro,type=bind", - "source=phlex-kilo-data,target=/root/.local/share/kilo,type=volume", - "source=${localEnv:HOME}/.podman-proxy/podman.sock,target=/tmp/podman.sock,type=bind", - "source=${localEnv:HOME}/.vscode-remote-user-data,target=/root/.vscode-server-insiders/data/User,type=bind" - ], "initializeCommand": "bash .devcontainer/ensure-repos.sh", "onCreateCommand": "bash .devcontainer/setup-repos.sh /workspaces", "postCreateCommand": "bash -lc 'bash .devcontainer/post-create.sh'", "customizations": { + "vscode": { "settings": { "terminal.integrated.defaultProfile.linux": "bash", @@ -88,7 +58,6 @@ "**/.venv/**": true, "**/py_virtual_env/**": true }, - "github.copilot-chat.usePreReleaseVersion": false, "kilocode.new.extraCaCerts": "" }, "extensions": [ @@ -99,7 +68,6 @@ "donjayamanne.githistory", "dotjoshjohnson.xml", "eamodio.gitlens", - "github.copilot-chat", "github.vscode-github-actions", "github.vscode-pull-request-github", "jebbs.plantuml", @@ -114,13 +82,11 @@ "ms-python.vscode-pylance", "ms-python.vscode-python-envs", "ms-vscode.cmake-tools", - "ms-vscode.cpptools", "ms-vscode.cpptools-extension-pack", "ms-vscode.cpptools-themes", "ms-vscode.hexeditor", "ms-vscode.live-server", "ms-vscode.makefile-tools", - "ms-vscode.vscode-websearchforcopilot", "redhat.vscode-yaml", "shd101wyy.markdown-preview-enhanced", "swyddfa.esbonio", diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml new file mode 100644 index 000000000..3c40de96d --- /dev/null +++ b/.devcontainer/docker-compose.yml @@ -0,0 +1,34 @@ +version: "3.8" + +services: + phlex-dev: + build: + context: . + dockerfile: Dockerfile + volumes: + - ..:/workspaces/phlex + - ../../phlex-coding-guidelines:/workspaces/phlex-coding-guidelines + - ../../phlex-design:/workspaces/phlex-design + - ../../phlex-examples:/workspaces/phlex-examples + - ../../phlex-spack-recipes:/workspaces/phlex-spack-recipes + - ${HOME}/.config/gh:/root/.config/gh:ro + - ${HOME}/.config/kilo:/root/.config/kilo + - ${HOME}/.gnupg:/root/.gnupg + - ${HOME}/.podman-proxy/podman.sock:/tmp/podman.sock + - phlex-kilo-data:/root/.local/share/kilo + - phlex-vscode-user-data:/root/.vscode-server-insiders/data/User + env_file: + - ${HOME}/.phlex-kilo.env + environment: + - CMAKE_GENERATOR=Ninja + - GH_CONFIG_DIR=/root/.config/gh + - DOCKER_HOST=unix:///tmp/podman.sock + - CONTAINER_HOST=unix:///tmp/podman.sock + - GNUPGHOME=/root/.gnupg + user: root + tty: true + stdin_open: true + +volumes: + phlex-kilo-data: + phlex-vscode-user-data: diff --git a/.devcontainer/ensure-repos.sh b/.devcontainer/ensure-repos.sh index 9303c9ddf..93bca0b2c 100755 --- a/.devcontainer/ensure-repos.sh +++ b/.devcontainer/ensure-repos.sh @@ -169,7 +169,6 @@ fi HEADROOM_PORT="${HEADROOM_PORT:-9797}" HEADROOM_RELAY_PORT=$(( HEADROOM_PORT + 10000 )) HEADROOM_LOCAL="127.0.0.1:${HEADROOM_PORT}" - if ss -tlnp 2>/dev/null | grep -q "127.0.0.1:${HEADROOM_PORT}"; then start_socat_relay \ "Headroom proxy" \ @@ -184,10 +183,22 @@ else fi # Ensure remaining source bind mount points exist. -ensure_bind_dir "$HOME/.aws" ensure_bind_dir "$HOME/.config/"{gh,kilo} ensure_bind_dir "$HOME/.gnupg" -ensure_bind_dir "$HOME/.kiro" -ensure_bind_dir "$HOME/.vscode-remote-user-data" + +# Generate .env file for the devcontainer to provide KILO_CONFIG_CONTENT +# to the VS Code plugin. This must be done on the host because the plugin +# needs this environment variable at server startup. +ENV_FILE="${HOME}/.phlex-kilo.env" +: > "${ENV_FILE}" + +if [ -f "${HOME}/.config/kilo/kilo.jsonc" ]; then + # Replace the baseURL for fnal-litellm specifically. + # The range /"fnal-litellm"/,/baseURL/ ensures we only modify the correct provider. + # We avoid stripping comments with sed to prevent corrupting URLs (https://), + # as the Kilo plugin's JSONC parser handles comments correctly. + KILO_CONFIG_CONTENT="$(perl -0777 -pe 's|("fnal-litellm".*?)"baseURL":\s*"[^"]*"|$1"baseURL": "http://host.docker.internal:'${HEADROOM_RELAY_PORT}'"|s; s/"[^"]*"(*SKIP)(*F)|\/\/.*//g; s/^\s+//gm; s/\n//g' "${HOME}/.config/kilo/kilo.jsonc")" + echo "KILO_CONFIG_CONTENT='${KILO_CONFIG_CONTENT}'" > "${ENV_FILE}" +fi echo "SUCCESS: .devcontainer/ensure-repos.sh completed successfully" diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index f7f35673f..c403ea07e 100755 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -17,15 +17,6 @@ EOF # installation on every rebuild. rm -f /root/.vscode-server-insiders/data/Machine/.installExtensionsMarker -# Set KILO_CONFIG_CONTENT for interactive shells. When KILO_CONFIG_CONTENT_DOCKER -# is non-empty (headroom local proxy via socat relay), use it so that the baseURL -# points at host.docker.internal rather than 127.0.0.1. Otherwise leave -# KILO_CONFIG_CONTENT unset so Kilo falls back to ~/.config/kilo/kilo.jsonc, -# which is bind-mounted from the host and may point at an external provider. -if [ -n "${KILO_CONFIG_CONTENT_DOCKER:-}" ]; then - printf 'export KILO_CONFIG_CONTENT=%q\n' "${KILO_CONFIG_CONTENT_DOCKER}" >> /root/.bashrc -fi - # Seed the Kilo Code auth token into the container-private data volume. # The volume is not shared with the host to avoid SQLite conflicts between # the Remote-SSH and devcontainer Kilo Code instances. The API key is @@ -99,6 +90,3 @@ if command -v prek >/dev/null 2>&1; then elif command -v pre-commit >/dev/null 2>&1; then pre-commit install || true fi - -# Install kiro-cli and set up shell integrations. -curl -fsSL https://cli.kiro.dev/install | bash diff --git a/.github/workflows/actionlint-check.yaml b/.github/workflows/actionlint-check.yaml index 961d59834..231468922 100644 --- a/.github/workflows/actionlint-check.yaml +++ b/.github/workflows/actionlint-check.yaml @@ -115,3 +115,29 @@ jobs: echo "::error::actionlint check failed. Please review the output above for details." exit 1 fi + + gating_check: + needs: setup + if: > + always() && ( + github.event_name == 'workflow_dispatch' || + inputs.skip-relevance-check || + needs.setup.outputs.has_changes == 'true' + ) + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout code + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: ${{ needs.setup.outputs.ref }} + path: ${{ needs.setup.outputs.checkout_path }} + repository: ${{ needs.setup.outputs.repo }} + persist-credentials: false + + - name: Verify gating-block consistency + env: + CHECKOUT_PATH: ${{ needs.setup.outputs.checkout_path }} + run: bash "${CHECKOUT_PATH}/scripts/check-gating-block.sh" "${CHECKOUT_PATH}/.github/workflows" diff --git a/.github/workflows/cmake-build.yaml b/.github/workflows/cmake-build.yaml index 04231a477..447cd845b 100644 --- a/.github/workflows/cmake-build.yaml +++ b/.github/workflows/cmake-build.yaml @@ -26,12 +26,12 @@ run-name: "${{ github.actor }} building and testing ${{ github.repository }}" Default (if empty): Run `gcc/none` required: false default: "" - perfetto-heap-profile: + perfetto_heap_profile: description: "Enable heap profiling for Perfetto runs" required: false type: boolean default: false - perfetto-cpu-profile: + perfetto_cpu_profile: description: "Enable CPU profiling for Perfetto runs" required: false type: boolean @@ -228,9 +228,9 @@ jobs: if: matrix.sanitizer == 'perfetto' working-directory: ${{ needs.setup.outputs.build_path }} env: - PERFETTO_HEAP_PROFILE: ${{ github.event.inputs.perfetto-heap-profile || 'false' }} + PERFETTO_HEAP_PROFILE: ${{ github.event.inputs.perfetto_heap_profile || 'false' }} PERFETTO_CPU_PROFILE: - ${{ (github.event.inputs.perfetto-cpu-profile != null && github.event.inputs.perfetto-cpu-profile) || true + ${{ (github.event.inputs.perfetto_cpu_profile != null && github.event.inputs.perfetto_cpu_profile) || true }} run: | . /entrypoint.sh diff --git a/.github/workflows/codeql-comment.yaml b/.github/workflows/codeql-comment.yaml index 6cd7271b6..d9bf07643 100644 --- a/.github/workflows/codeql-comment.yaml +++ b/.github/workflows/codeql-comment.yaml @@ -37,7 +37,7 @@ jobs: if: success() && hashFiles('pr_comment_data/pr_number.txt') != '' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: - github-token: ${{ secrets.WORKFLOW_PAT }} + github-token: ${{ github.token }} script: | const fs = require('fs'); const path = require('path'); diff --git a/docs/dev/WORKFLOW_CALLS.md b/docs/dev/WORKFLOW_CALLS.md new file mode 100644 index 000000000..42cd68dbd --- /dev/null +++ b/docs/dev/WORKFLOW_CALLS.md @@ -0,0 +1,606 @@ +# Reusable GitHub Actions Workflows + +This document describes all reusable GitHub Actions workflows in the phlex repository. These workflows have `workflow_call` triggers and can be invoked by other workflows. + +## Trigger Model for Fix Workflows + +The `*-fix` workflows (`clang-format-fix`, `clang-tidy-fix`, `cmake-format-fix`, `header-guards-fix`, `jsonnet-format-fix`, `markdown-fix`, `python-fix`, `yaml-fix`) follow a specific trigger model: + +- **Triggers**: `issue_comment` (gated to OWNER/COLLABORATOR/MEMBER), `workflow_dispatch`, and `workflow_call` +- **NOT triggered by**: `pull_request` events + +These workflows always run in the base-repo context to ensure they have write access to push fixes back to branches. + +### Why WORKFLOW_PAT is Required + +Commits pushed to re-trigger CI cannot use `GITHUB_TOKEN` because GitHub suppresses workflow runs triggered by `GITHUB_TOKEN` pushes to prevent infinite recursion loops. The `WORKFLOW_PAT` (a personal access token with repository scope) is required to push commits and trigger subsequent CI runs. + +## Canonical Base-Input Contract + +All check and fix workflows share a common base-input contract passed through the `Framework-R-D/action-workflow-setup` action: + +| Input | Type | Required | Description | +| ------- | ------ | ---------- | ------------- | +| `checkout-path` | string | No | Path to check out code to | +| `skip-relevance-check` | boolean | No | Bypass relevance check (default: `false`) | +| `ref` | string | No | The branch, ref, or SHA to checkout | +| `repo` | string | No | The repository to checkout from | +| `pr-base-sha` | string | No | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | Head SHA of the PR for relevance check | + +## Check Workflows + +Check workflows are triggered by `pull_request`, `push`, `workflow_dispatch`, and `workflow_call`. They analyze code without making changes. + +### Actionlint Check + +**File**: `.github/workflows/actionlint-check.yaml` + +Validates GitHub Actions workflow files using actionlint. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/actionlint-check.yaml +with: + checkout-path: . + skip-relevance-check: false + ref: refs/heads/main +``` + +### Clang-Format Check + +**File**: `.github/workflows/clang-format-check.yaml` + +Checks C++ source files for formatting compliance using clang-format 20. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/clang-format-check.yaml +with: + checkout-path: . + skip-relevance-check: false +``` + +### CMake Format Check + +**File**: `.github/workflows/cmake-format-check.yaml` + +Checks CMake files for formatting compliance using gersemi. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/cmake-format-check.yaml +with: + checkout-path: . +``` + +### Header Guards Check + +**File**: `.github/workflows/header-guards-check.yaml` + +Checks C++ header files for proper include guard patterns. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/header-guards-check.yaml +with: + checkout-path: . +``` + +### Jsonnet Format Check + +**File**: `.github/workflows/jsonnet-format-check.yaml` + +Checks Jsonnet files for formatting compliance using jsonnetfmt v0.22.0. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/jsonnet-format-check.yaml +with: + checkout-path: . +``` + +### Markdown Check + +**File**: `.github/workflows/markdown-check.yaml` + +Validates Markdown files using markdownlint-cli2, excluding CHANGELOG.md. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/markdown-check.yaml +with: + checkout-path: . +``` + +### Python Check + +**File**: `.github/workflows/python-check.yaml` + +Runs ruff for linting and formatting checks, and MyPy for type checking. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/python-check.yaml +with: + checkout-path: . +``` + +### YAML Check + +**File**: `.github/workflows/yaml-check.yaml` + +Validates YAML files using yamllint. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/yaml-check.yaml +with: + checkout-path: . +``` + +### Clang-Tidy Check + +**File**: `.github/workflows/clang-tidy-check.yaml` + +Runs clang-tidy on C++ source files. Also accepts `issue_comment` triggers. + +#### Inputs + +This workflow does not expose inputs via `workflow_call`. It is triggered by `issue_comment`, `pull_request`, `push`, or `workflow_dispatch`. + +#### Example Invocation + +```yaml +uses: ./.github/workflows/clang-tidy-check.yaml +with: + # Uses issue_comment to trigger via: @reponamebot tidy-check +``` + +### CMake Build + +**File**: `.github/workflows/cmake-build.yaml` + +Configures and builds the project with CMake, then runs tests. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `build-path` | string | No | N/A | Path for build artifacts | +| `skip-relevance-check` | boolean | No | `false` | Bypass relevance check | +| `build-combinations` | string | No | N/A | Space- or comma-separated list of build combinations to run | +| `ref` | string | No | N/A | The branch, ref, or SHA to checkout | +| `repo` | string | No | N/A | The repository to checkout from | +| `pr-base-sha` | string | No | N/A | Base SHA of the PR for relevance check | +| `pr-head-sha` | string | No | N/A | Head SHA of the PR for relevance check | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/cmake-build.yaml +with: + checkout-path: . + build-path: build + build-combinations: "gcc/none clang/asan" +``` + +### Coverage + +**File**: `.github/workflows/coverage.yaml` + +Generates code coverage reports using GCC or Clang. + +#### Inputs + +This workflow does not expose inputs via `workflow_call`. It is triggered by `issue_comment`, `pull_request`, `push`, `schedule`, or `workflow_dispatch`. + +#### Example Invocation + +```yaml +uses: ./.github/workflows/coverage.yaml +with: + # Uses issue_comment to trigger via: @reponamebot coverage +``` + +## Fix Workflows + +Fix workflows are triggered by `issue_comment`, `workflow_dispatch`, or `workflow_call`. They modify code and push changes back to branches. + +### Clang-Format Fix + +**File**: `.github/workflows/clang-format-fix.yaml` + +Applies clang-format 20 to C++ source files. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `ref` | string | Yes | N/A | The branch name to checkout and push fixes to | +| `repo` | string | Yes | N/A | The repository to checkout from | +| `skip-comment` | boolean | No | `false` | Skip posting PR comments | + +#### Outputs + +| Output | Description | +| -------- | ------------- | +| `changes` | Whether any fixes were applied | +| `pushed` | Whether the fixes were pushed to the remote branch | +| `commit_sha` | The full commit SHA of the applied fixes | +| `commit_sha_short` | The short commit SHA of the applied fixes | +| `patch_name` | Name of the patch file if fixes could not be pushed | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/clang-format-fix.yaml +with: + checkout-path: . + ref: refs/heads/my-feature + repo: ${{ github.repository }} + skip-comment: false +``` + +### CMake Format Fix + +**File**: `.github/workflows/cmake-format-fix.yaml` + +Applies gersemi formatting to CMake files. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `ref` | string | Yes | N/A | The branch name to checkout and push fixes to | +| `repo` | string | Yes | N/A | The repository to checkout from | +| `skip-comment` | boolean | No | `false` | Skip posting PR comments | + +#### Outputs + +| Output | Description | +| -------- | ------------- | +| `changes` | Whether any fixes were applied | +| `pushed` | Whether the fixes were pushed to the remote branch | +| `commit_sha` | The full commit SHA of the applied fixes | +| `commit_sha_short` | The short commit SHA of the applied fixes | +| `patch_name` | Name of the patch file if fixes could not be pushed | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/cmake-format-fix.yaml +with: + checkout-path: . + ref: refs/heads/my-feature + repo: ${{ github.repository }} +``` + +### Header Guards Fix + +**File**: `.github/workflows/header-guards-fix.yaml` + +Applies header guard fixes to C++ header files. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `ref` | string | Yes | N/A | The branch name to checkout and push fixes to | +| `repo` | string | Yes | N/A | The repository to checkout from | +| `skip-comment` | boolean | No | `false` | Skip posting PR comments | + +#### Outputs + +| Output | Description | +| -------- | ------------- | +| `changes` | Whether any fixes were applied | +| `pushed` | Whether the fixes were pushed to the remote branch | +| `commit_sha` | The full commit SHA of the applied fixes | +| `commit_sha_short` | The short commit SHA of the applied fixes | +| `patch_name` | Name of the patch file if fixes could not be pushed | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/header-guards-fix.yaml +with: + checkout-path: . + ref: refs/heads/my-feature + repo: ${{ github.repository }} +``` + +### Jsonnet Format Fix + +**File**: `.github/workflows/jsonnet-format-fix.yaml` + +Applies jsonnetfmt formatting to Jsonnet files. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `ref` | string | Yes | N/A | The branch name to checkout and push fixes to | +| `repo` | string | Yes | N/A | The repository to checkout from | +| `skip-comment` | boolean | No | `false` | Skip posting PR comments | + +#### Outputs + +| Output | Description | +| -------- | ------------- | +| `changes` | Whether any fixes were applied | +| `pushed` | Whether the fixes were pushed to the remote branch | +| `commit_sha` | The full commit SHA of the applied fixes | +| `commit_sha_short` | The short commit SHA of the applied fixes | +| `patch_name` | Name of the patch file if fixes could not be pushed | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/jsonnet-format-fix.yaml +with: + checkout-path: . + ref: refs/heads/my-feature + repo: ${{ github.repository }} +``` + +### Markdown Fix + +**File**: `.github/workflows/markdown-fix.yaml` + +Applies markdownlint-cli2 formatting fixes to Markdown files. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `ref` | string | Yes | N/A | The branch name to checkout and push fixes to | +| `repo` | string | Yes | N/A | The repository to checkout from | +| `skip-comment` | boolean | No | `false` | Skip posting PR comments | + +#### Outputs + +| Output | Description | +| -------- | ------------- | +| `changes` | Whether any fixes were applied | +| `pushed` | Whether the fixes were pushed to the remote branch | +| `commit_sha` | The full commit SHA of the applied fixes | +| `commit_sha_short` | The short commit SHA of the applied fixes | +| `patch_name` | Name of the patch file if fixes could not be pushed | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/markdown-fix.yaml +with: + checkout-path: . + ref: refs/heads/my-feature + repo: ${{ github.repository }} +``` + +### Python Fix + +**File**: `.github/workflows/python-fix.yaml` + +Applies ruff formatting and lint fixes to Python files. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `ref` | string | Yes | N/A | The branch name to checkout and push fixes to | +| `repo` | string | Yes | N/A | The repository to checkout from | +| `skip-comment` | boolean | No | `false` | Skip posting PR comments | + +#### Outputs + +| Output | Description | +| -------- | ------------- | +| `changes` | Whether any fixes were applied | +| `pushed` | Whether the fixes were pushed to the remote branch | +| `commit_sha` | The full commit SHA of the applied fixes | +| `commit_sha_short` | The short commit SHA of the applied fixes | +| `patch_name` | Name of the patch file if fixes could not be pushed | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/python-fix.yaml +with: + checkout-path: . + ref: refs/heads/my-feature + repo: ${{ github.repository }} +``` + +### YAML Fix + +**File**: `.github/workflows/yaml-fix.yaml` + +Applies prettier formatting to YAML files. + +#### Inputs + +| Input | Type | Required | Default | Description | +| ------- | ------ | ---------- | --------- | ------------- | +| `checkout-path` | string | No | N/A | Path to check out code to | +| `ref` | string | Yes | N/A | The branch name to checkout and push fixes to | +| `repo` | string | Yes | N/A | The repository to checkout from | +| `skip-comment` | boolean | No | `false` | Skip posting PR comments | + +#### Outputs + +| Output | Description | +| -------- | ------------- | +| `changes` | Whether any fixes were applied | +| `pushed` | Whether the fixes were pushed to the remote branch | +| `commit_sha` | The full commit SHA of the applied fixes | +| `commit_sha_short` | The short commit SHA of the applied fixes | +| `patch_name` | Name of the patch file if fixes could not be pushed | + +#### Example Invocation + +```yaml +uses: ./.github/workflows/yaml-fix.yaml +with: + checkout-path: . + ref: refs/heads/my-feature + repo: ${{ github.repository }} +``` + +### Clang-Tidy Fix + +**File**: `.github/workflows/clang-tidy-fix.yaml` + +Applies clang-tidy fixes via clang-apply-replacements. Accepts an optional comma-separated list of checks to apply. + +#### Inputs + +This workflow does not expose inputs via `workflow_call`. It is triggered by `issue_comment` or `workflow_dispatch`. + +#### Example Invocation + +```yaml +uses: ./.github/workflows/clang-tidy-fix.yaml +with: + # Uses issue_comment to trigger via: @reponamebot tidy-fix + # Or workflow_dispatch with inputs for ref and tidy-checks +``` + +## Composite Workflows + +### Format All + +**File**: `.github/workflows/format-all.yaml` + +Calls all individual formatter workflows (`clang-format-fix`, `cmake-format-fix`, `header-guards-fix`, `jsonnet-format-fix`, `markdown-fix`, `python-fix`, `yaml-fix`) in sequence and aggregates the results into a single PR comment. + +This workflow is triggered only by `issue_comment` (gated to OWNER/COLLABORATOR/MEMBER). + +#### Example Invocation + +```yaml +uses: ./.github/workflows/format-all.yaml +with: + # Triggered via issue_comment: @reponamebot format +``` + +## Usage Guidelines + +1. **Check workflows**: Use `workflow_call` to invoke check workflows from other workflows. They are also triggered automatically on `pull_request` and `push` events. + +2. **Fix workflows**: Use `workflow_call` to invoke fix workflows programmatically, or use the bot commands in PR comments: + - `@reponamebot clang-fix` + - `@reponamebot cmake-fix` + - `@reponamebot header-guards-fix` + - `@reponamebot jsonnet-fix` + - `@reponamebot markdown-fix` + - `@reponamebot python-fix` + - `@reponamebot yaml-fix` + - `@reponamebot format` (runs all formatters) + +3. **When invoking via `workflow_call`**: Always pass `ref`, `repo`, and `checkout-path`. Set `skip-comment: true` when calling from another fixed workflow to avoid duplicate comments. + +4. **Triggering fix workflows**: Use `workflow_dispatch` for manual triggering, or use the `issue_comment` trigger via bot commands in PRs. diff --git a/docs/dev/generating-executing-kilo-plans.md b/docs/dev/generating-executing-kilo-plans.md new file mode 100644 index 000000000..3e537d79d --- /dev/null +++ b/docs/dev/generating-executing-kilo-plans.md @@ -0,0 +1,291 @@ +# Generating & Executing Kilo Plans + +## Purpose + +Explain how to produce a structured, machine-readable JSON execution plan with the **genplan** agent and how to run that plan with the **execute** orchestrator. The process supports idempotent re‑runs, explicit model selection, built‑in error handling, and in‑flight re‑planning. + +> **Why `genplan`, not `plan`?** `plan` is a **reserved native agent name** in the +> Kilo CLI: it is hard-wired to the built-in Markdown "Plan mode", which researches +> the goal, asks you to "Finalize and save the plan" or "Continue refining", and +> writes a human-readable Markdown plan to `.kilo/plans/*.md`. That native behavior +> overrides a custom agent's system prompt and permissions **by name**, so a custom +> agent named `plan` cannot emit JSON. The JSON planner is therefore named +> `genplan`. Other reserved names to avoid for custom agents: `build`, `code`, +> `ask`, `architect`, `explore`, `general`, `orchestrator`, `debug`, `scout`. + +## 1. Generate a plan + +1. **Define the goal** – a short, high‑level statement (e.g. "Add a dark‑mode toggle to the web UI"). +2. **Invoke the planner** with `kilo run`. The `genplan` agent is configured with + `mode: all`, so it can be selected as a primary agent directly. The prompt is + passed as a positional argument (there is no `--prompt` option), and + `--title` optionally names the session (it defaults to a truncated prompt): + + ```bash + kilo run --agent genplan --title "Generate plan for " "" + ``` + + Replace `` with your actual objective. +3. The planner returns JSON following the schema in `agent/genplan.md`. Key fields: + * `goal` – original request + * `idempotent` – true if the whole plan may be safely re‑run + * `steps` – ordered array where each step contains: + * `id` – unique identifier (e.g. `step-1`) + * `description` + * `model` – any builtin agent shortcut (`reason`, `ask`, `chat`, `code`, `coder-qwen`) **or** a full `fnal-litellm` provider/model identifier such as `fnal-litellm/openai/gpt-oss-120b` or `fnal-litellm/azure/claude-opus-4-8`. Only `fnal-litellm` provider models are permitted. + * `input` / `output` + * `gate`, `parallel`, `retry`, `max_retries`, `on_error`, `self_check` + +## 2. Example output + +```json +{ + "goal": "Add a dark‑mode toggle to the web UI", + "idempotent": true, + "steps": [ + { + "id": "step-1", + "description": "Create design spec for the toggle component", + "model": "reason", + "input": null, + "output": "design-spec.md", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 2, + "on_error": "abort", + "self_check": "file_exists('design-spec.md')" + }, + { + "id": "step-2", + "description": "Implement the Toggle React component", + "model": "coder-qwen", + "input": "design-spec.md", + "output": "src/components/Toggle.jsx", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "replan", + "self_check": "eslint_passes('src/components/Toggle.jsx')" + } + // …additional steps + ] +} +``` + +## 3. Execute the plan + +Use the orchestrator (the primary `execute` agent) to consume the JSON. As with +the planner, the message is a positional argument and `--title` is optional: + +```bash +kilo run --agent execute --title "Execute generated plan" "$(cat plan.json)" +``` + +The orchestrator: + +* Reads each step in order. +* Calls the specified model/sub‑agent (`reason`, `code`, `ask`, `chat`, `coder-qwen`, or `genplan`). +* Runs `self_check` after completion. +* Honors `gate` (manual approval), `parallel` (concurrent execution), and retry logic. +* Stops, continues, falls back, or re‑plans based on `on_error`. + +Because the plan is marked `idempotent`, you may safely re‑run the whole plan—or only the failing steps—without side‑effects, provided the underlying actions themselves are idempotent (e.g., use upserts, check‑before‑write). + +### In‑flight re‑planning + +A plan is not frozen once execution begins. When a step declares +`on_error: replan` and fails after exhausting `max_retries`, the orchestrator +suspends execution and re‑invokes the `genplan` agent as a sub‑agent to revise the +*remaining* work, using the accumulated outputs and the failure context. This is +what the `mode: all` setting on the `genplan` agent enables: the same agent serves +both as the directly‑invocable planner (section 1) and as the re‑planning +sub‑agent dispatched by `execute`. + +Re‑planning preserves already‑completed steps verbatim so downstream +dependencies stay satisfied; the planner emits only new or replacement steps +with fresh, non‑colliding `id`s. The orchestrator records each re‑planning event +(the triggering step, the reason, and the new step ids) in its final JSON report +so the revision history is auditable. + +For a human‑in‑the‑loop revision, pause at a `gate: true` step, run +`kilo run --agent genplan ""` to produce an updated plan, and hand the +result back to a fresh `execute` run. + +## 4. Model‑selection policy + +* Prefer builtin agent shortcuts for simplicity. +* For performance‑critical or accuracy‑critical tasks, specify an explicit `fnal-litellm` model (e.g. `fnal-litellm/azure/claude-opus-4-8`, `fnal-litellm/azure/gpt_5.3-codex`). +* Only `fnal-litellm` provider models are permitted; the `genplan` and `execute` agents reject models from other providers. +* The planner defaults to the free/open‑weight model (`fnal-litellm/openai/gpt-oss-120b`) unless a higher‑capability model is explicitly required. + +### Planner vs orchestrator model choice + +The two agents have different demands, and observed runs justify different +defaults: + +* **`genplan` (planning)** decomposes a goal into JSON in essentially one shot. + The free/open‑weight `fnal-litellm/openai/gpt-oss-120b` handles this well and + remains the default — planning is a low‑tool‑count, one‑shot task, and plan + defects are cheap to catch before they do harm (a `gate: true` step invites + human review, and `on_error: replan` lets the plan self‑correct during + execution). A run of `genplan` on `fnal-litellm/azure/claude-opus-4-8` + produced a correct plan but was measurable overkill: it engaged **zero** + reasoning tokens, confirming the task did not stress a frontier model. Keep + `gpt-oss-120b` as the planner default and escalate only on observed + inadequacy. + + Practical complexity ceiling for the default planner: + + * **`gpt-oss-120b` is sufficient** when the goal is backed by a written + spec/markdown plan to transcribe, or decomposes into roughly ≤ 15 mostly + linear steps with clear inputs/outputs (the git‑ai‑commit optimization plan + is a representative example — 11 steps, all delegated to `coder-qwen`). + * **Consider `fnal-litellm/azure/claude-sonnet-4-6`** for larger DAGs (~15–30 + steps) with non‑trivial cross‑step dependencies, parallelization, or gating + that the plan itself must reason about. + * **Reserve `fnal-litellm/azure/claude-opus-4-8`** for the rare case where + planning is itself hard reasoning — many interacting constraints with **no** + authoritative spec to transcribe, so the planner must derive the design. + This did not apply to the git‑ai‑commit plan. +* **`execute` (orchestration)** must dispatch substantial steps to a sub‑agent + via the `Task` tool, track dependencies and gates, apply trivial edits itself + only when spawning a sub‑agent would cost more than the edit, and recover when + a sub‑agent runs short of its own budget. Weaker models fail this in the + opposite direction: they "help" by implementing whole steps directly, which + burns the orchestrator's step budget and causes premature step‑limit + exhaustion. Observed directly here: the same 11‑step plan run headless on + `gpt-oss-120b` stalled at ~step 6 (it edited files itself instead of + delegating), whereas a forked continuation on + `fnal-litellm/azure/claude-sonnet-4-6` completed all steps — delegating the + test‑ and doc‑authoring, applying the one‑line edits inline, and debugging a + sub‑agent that had run out of steps. For that reason the orchestrator defaults + to `claude-sonnet-4-6`. Reserve `fnal-litellm/azure/claude-opus-4-8` for + unusually large or tangled plans where Sonnet‑level coordination is + insufficient; it is roughly 5× the per‑token cost and rarely necessary just to + orchestrate. +* The per‑step `model` fields stay on the cheapest capable option + (`coder-qwen` for code) regardless of which model runs the orchestrator — the + orchestrator's model governs coordination, not the implementation work. + +## 5. Verify with `prek` + +After creating or editing the guide, run the pre‑commit hook to ensure compliance: + +```bash +prek run --all-files +``` + +`prek` checks for trailing whitespace, multiple final newlines, and basic markdown linting. This file follows those rules out‑of‑the‑box. + +--- +*Generated by the Kilo planner and orchestrator.* + +## 6. Resuming an interrupted execution + +Orchestration runs can stop before the plan is complete. The most common cause +is **step‑limit exhaustion**: the `execute` agent has a finite per‑session step +budget (`steps:` in `agent/execute.md`), and when it is reached the agent emits +a partial summary and stops. Other causes are an `on_error: abort` step, a +`gate: true` checkpoint awaiting human review, or a manual interrupt. In every +case the orchestrator should leave an auditable resume point naming the last +completed step id and the next pending step id. + +There are two ways to resume, and the correct choice depends on the plan's +`idempotent` flag. + +### Option A — continue the interrupted session (preferred) + +Continuing replays the existing transcript, so the orchestrator already knows +which steps completed regardless of the `idempotent` flag. Use `--continue` +(`-c`) for the **most recent** session, or `--session ` to target a +**specific** session by id (list ids with `kilo session`): + +```bash +# resume the most recent session +kilo run --agent execute -c "continue executing the remaining plan steps" + +# or resume a specific session by id +kilo run --agent execute --session ses_XXXXXXXX "continue executing the remaining plan steps" +``` + +Add `--fork` to branch a new session from the interrupted one (leaving the +original transcript untouched) when you want to retry without overwriting the +original log. This is the recommended path when the plan is `idempotent: false`, +because re‑running completed steps in a fresh session could repeat +non‑idempotent side effects. + +### Option B — start a fresh session + +Only safe when the plan is `idempotent: true`, **or** when you first trim the +JSON plan to the not‑yet‑completed steps (and re‑point their `input` +dependencies at artifacts that already exist on disk): + +```bash +kilo run --agent execute --title "Resume execution" "$(cat plan-remaining.json)" +``` + +For an `idempotent: true` plan you may pass the original plan unchanged; the +orchestrator treats steps whose `id` matches a completed step as done via +step‑ID matching. For an `idempotent: false` plan (such as the git‑ai‑commit +optimization plan, where edits are not safe to re‑apply), do **not** re‑submit +the full original plan to a fresh session — either continue the session +(Option A) or hand the orchestrator a plan containing only the remaining steps. + +### Resuming across a `gate` + +When execution paused at a `gate: true` step, review the gated artifact, then +resume with Option A (`--continue`/`--session`) and instruct the orchestrator to +proceed past the approved gate. If the review requires plan changes, run +`kilo run --agent genplan ""` +to obtain a revised plan and feed that to a fresh `execute` run. + +Both approaches respect `on_error: replan`: if a step fails again, the planner +is invoked to produce a revised plan. For quick iteration `--continue` is most +convenient; a fresh session on a trimmed remaining‑steps plan gives a clean log. + +## 7. Interactive vs non‑interactive invocation + +`kilo run` has two operating modes, selected by the `--interactive`/`-i` flag. + +* **Non‑interactive (default):** `kilo run --agent genplan …` or + `kilo run --agent execute …` runs headless. The agent receives the full + prompt, performs its work, and returns a single result. Permission prompts + block the run unless you pass `--auto` (auto‑approve all permissions) or + `--dangerously-skip-permissions` (auto‑approve only permissions that are not + explicitly denied). Use this mode for pipelines and for the normal + generate‑then‑execute flow. +* **Interactive (`-i`):** switches to the split‑footer TUI, where you converse + with the agent turn by turn, can steer or correct it, and approve permission + prompts as they arise. This does **not** insert an automatic pause after every + reasoning step; it simply keeps you in the loop so you can intervene between + turns. Use it when the plan touches ambiguous code, when you want to watch a + large diff before an escalation, or when debugging why the orchestrator made + an unexpected choice. `--replay` (optionally `--replay-limit N`) reprints prior + session history when resuming interactively. + +### Pros and cons + +* Non‑interactive is faster and scriptable but gives no chance to course‑correct + mid‑run; a bad early decision runs to completion or to the step limit. +* Interactive is slower and needs a human present but catches wrong model + choices, runaway edits, or context‑size problems before they consume the + budget. + +### Switching modes when problems occur + +* If a headless `execute` run stalls, loops, or picks an unexpected model, + resume it interactively: `kilo run --agent execute -i -c "…"` (or with + `--session `), watch the next few turns, and steer. +* `--model provider/model` overrides the **agent's own** model for that run + (e.g. force the orchestrator onto `fnal-litellm/azure/claude-opus-4-8` for a + hard coordination job). It does not override the per‑step `model` fields in + the plan — those are honored by the orchestrator when it dispatches each + sub‑agent. To change a step's model, edit the plan. + +You can switch between modes on the fly without changing the underlying plan or +agent definitions. + +--- +*Generated by the Kilo planner and orchestrator.* diff --git a/docs/dev/workflow-audit-implementation-plan.json b/docs/dev/workflow-audit-implementation-plan.json new file mode 100644 index 000000000..eeb59901d --- /dev/null +++ b/docs/dev/workflow-audit-implementation-plan.json @@ -0,0 +1,97 @@ +{ + "goal": "Implement recommendations from workflow-audit-report.md", + "idempotent": false, + "steps": [ + { + "id": "step-1", + "description": "Create documentation file WORKFLOW_CALLS.md listing all reusable workflow names, required inputs, and example invocations", + "model": "coder-qwen", + "input": null, + "output": "WORKFLOW_CALLS.md created in docs/dev/", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 2, + "on_error": "abort", + "self_check": "file exists and contains markdown headings for each workflow" + }, + { + "id": "step-2", + "description": "Standardize boolean workflow inputs to snake_case (e.g., skip-relevance-check) across all .github/workflows/*.yaml files", + "model": "coder-qwen", + "input": "List of workflow YAML files via grep pattern '\\.yaml' in .github/workflows", + "output": "All boolean inputs renamed consistently; corresponding references updated", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 3, + "on_error": "continue", + "self_check": "grep for camelCase or kebab-case boolean inputs yields no results" + }, + { + "id": "step-3", + "description": "Add missing description fields for 'ref' input and other extra inputs in all workflow_dispatch sections", + "model": "coder-qwen", + "input": "Search for workflow_dispatch blocks lacking description for inputs", + "output": "All inputs in workflow_dispatch have a 'description' field", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 2, + "on_error": "continue", + "self_check": "grep for 'workflow_dispatch' sections where 'inputs' entries miss a 'description' key returns zero" + }, + { + "id": "step-4", + "description": "Add a lightweight lint/assertion script (e.g., scripts/check-gating-block.sh) that verifies the shared if‑gating block is identical across all workflows and fails CI if drift is detected", + "model": "coder-qwen", + "input": null, + "output": "New script added and referenced in CI workflow (e.g., actionlint job)", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 2, + "on_error": "abort", + "self_check": "Run script locally; exits 0 only when all workflows contain identical gating block" + }, + { + "id": "step-5", + "description": "Update permissions for workflows that currently over‑grant to use fine‑grained token scoped to Contents: write and Pull‑requests: write (replace classic PAT where possible)", + "model": "coder-qwen", + "input": "Identify workflows using secrets.WORKFLOW_PAT where only comment steps need it", + "output": "Permissions blocks simplified; comment‑only steps migrated to workflow_run using GITHUB_TOKEN, PAT retained only for commit‑push paths", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 2, + "on_error": "continue", + "self_check": "grep for 'secrets.WORKFLOW_PAT' in comment‑only jobs returns none" + }, + { + "id": "step-6", + "description": "Add CI job to run the gating‑block lint script and enforce variable‑naming consistency", + "model": "coder-qwen", + "input": null, + "output": "CI workflow (e.g., .github/workflows/ci.yaml) includes new job 'gating_check' that fails on drift", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 1, + "on_error": "abort", + "self_check": "CI config contains job named gating_check" + }, + { + "id": "step-7", + "description": "Run pre‑commit hooks and CI locally to validate changes; commit all modifications in a single atomic commit", + "model": "coder-qwen", + "input": null, + "output": "All tests pass, CI passes, and repository is clean", + "gate": false, + "parallel": false, + "retry": 0, + "max_retries": 1, + "on_error": "abort", + "self_check": "pre‑commit run --all-files returns success and ctest passes" + } + ] +} diff --git a/docs/dev/workflow-audit-report.md b/docs/dev/workflow-audit-report.md new file mode 100644 index 000000000..b7a5634f9 --- /dev/null +++ b/docs/dev/workflow-audit-report.md @@ -0,0 +1,394 @@ +# Phlex & Action Repositories – Workflow & Reusable-Action Audit + +*Prepared for the `phlex` monorepo (primary) and the `action/*` sibling repos.* + +--- + +## Table of Contents + +1. [Scope & Methodology](#scope--methodology) +2. [Structural Consistency & Naming](#1-structural-consistency--naming) +3. [Trigger Correctness (`workflow_dispatch` / `workflow_call`)](#2-trigger-correctness) +4. [Fork-PR Compatibility](#3-fork-pr-compatibility) +5. [Permissions & `WORKFLOW_PAT` Usage](#4-permissions--workflow_pat-usage) +6. [Action-Repo Workflow Parity](#5-action-repo-workflow-parity) +7. [Prioritized Recommendations](#prioritized-recommendations) +8. [Appendix – Quick Reference Tables](#appendix--quick-reference-tables) + +--- + +## Scope & Methodology + +| Item | What was examined | Tools used | +| ------ | ------------------- | ----------- | +| **Phlex workflows** | All 27 YAML files under `phlex/.github/workflows/` | `grep`, `read`, `bash` to list files | +| **Action reusable-actions** | `action/*/action.yaml` + CI workflows under `action/*/.github/workflows/` | Same tooling | +| **Cross-repo parity** | Comparison of each phlex workflow with the copy-step used for the action repos. | Manual diff of key sections | +| **Triggers, permissions, PATs** | Specific patterns (`workflow_dispatch`, `workflow_call`, `permissions:`, `secrets.WORKFLOW_PAT`). | `grep` across the repo tree | +| **Fork handling** | Workflows that post PR comments or artifacts (clang-tidy, coverage, format-fix, etc.). | Reviewed explicit `if:` conditions and auxiliary “-report” workflows | + +> **Model size note** – The structural pattern-matching in this report (naming, input inventory, `if:`-gating comparison) required only a few hundred lines of YAML parsing. However, an earlier revision of this report drew incorrect conclusions about trigger semantics, the `GITHUB_TOKEN` CI-recursion barrier, and cross-file YAML-anchor reuse in GitHub Actions — precisely the Actions-specific reasoning that pattern matching does not capture. Sections 3, 4, and the Prioritized Recommendations have been corrected accordingly. Reviewers should treat Actions-runtime semantics claims as requiring verification against the actual trigger blocks, not inferred from surface structure. + +--- + +## 1. Structural Consistency & Naming + +### 1.1 Common “base” inputs (present in most `workflow_call` sections) + +| Input | Description | Present in | +| ------- | ------------- | ------------ | +| `checkout-path` | Path to checkout the repo | **All** workflow-call enabled files | +| `skip-relevance-check` | Bypass relevance detection | **All** | +| `pr-base-sha` & `pr-head-sha` | Relevance-check SHAs | **All** | +| `ref` | Branch / SHA to check out | **All** | +| `repo` | Repository to checkout | **All** | + +#### Missing / Inconsistent Inputs + +| Workflow | Missing input(s) | Impact | +| ---------- | ------------------ | -------- | +| `clang-format-check.yaml`, `clang-format-fix.yaml`, `cmake-format-check.yaml`, `cmake-format-fix.yaml`, `jsonnet-format-*.yaml` | `build-path` (present only in `cmake-build.yaml` and a few others) | Consumers that need a custom build directory cannot reuse these without fork-editing. | +| `python-check.yaml`, `python-fix.yaml` | `build-combinations` (only in `cmake-build.yaml`) | Not applicable – fine; but naming differs (`build-combinations` vs. none). | +| `coverage.yaml` | No `skip-relevance-check` (uses its own gating) | Slightly divergent semantics; may cause confusion in docs. | + +### 1.2 Job & Stage Naming + +| Semantic group | Standard name in phlex | Outliers | +| ---------------- | ------------------------ | ---------- | +| Setup | `setup` | None | +| Build matrix generation | `generate-matrix` | `generate-matrix` used only in `cmake-build.yaml`; other checks don’t need a matrix. | +| Build / test | `build` | `clang-tidy-check` & `clang-tidy-fix` use `clang-tidy-check` (clear) – fine. | +| Reporting / comment | `build-complete` / `report` | `clang-tidy-report.yaml` uses a separate workflow (`report`) – consistent. | +| Linting | `yaml-check`, `markdown-check`, `clang-format-check`, `clang-tidy-check` | Consistent. | +| Format-fix | `yaml-fix`, `markdown-fix`, `clang-format-fix`, `clang-tidy-fix` | Consistent. | +| Coverage | `coverage` (single job) | No divergence. | + +**Variable Naming** – Across all workflows, environment variables are capitalised (e.g., `BUILD_TYPE`, `CICOLOR_FORCE`). The only exception is the `perfetto` flags inside `cmake-build.yaml` that expose lower-case env vars (`perfetto-heap-profile`, `perfetto-cpu-profile`) **only** for `workflow_dispatch`; they are referenced via `${{ github.event.inputs.perfetto-heap-profile }}` – acceptable but broken consistency with the other boolean inputs (`skip-relevance-check`). + +--- + +## 2. Trigger Correctness + +### 2.1 `workflow_dispatch` + +| Workflow | Inputs defined | Default values | Comments | +| ---------- | ---------------- | ---------------- | ---------- | +| All check/fix workflows (`clang-*`, `cmake-*`, `jsonnet-*`, `markdown-*`, `python-*`, `yaml-*`, `coverage`) | `ref` (string) – **no default** | none | Good – allows explicit ref override. | +| `cmake-build.yaml` | **Additional** inputs `build-combinations`, `perfetto-heap-profile`, `perfetto-cpu-profile` | `build-combinations`: `""` (empty) → defaults to `gcc/none` in script; `perfetto-*`: boolean defaults (false/true) | Consistent with the underlying build script. | +| `coverage.yaml` | Several extra inputs (`phlex-coverage-compiler`, `phlex-enable-form`, etc.) | Mostly empty strings or booleans with defaults | Correct – matched by later `if` statements. | + +**Issue:** Some `workflow_dispatch` blocks omit a `description` for the `ref` input (e.g., `jsonnet-format-fix.yaml` – description present, but `markdown-fix.yaml` does not). While not functional, it reduces self-documentation. + +### 2.2 `workflow_call` + +All 13 workflows that expose `workflow_call` share the same core set of inputs (see 1.1). The only divergences are: + +| Workflow | Extra input(s) | +| ---------- | ---------------- | +| `cmake-build.yaml` | `build-combinations` | +| `clang-tidy-fix.yaml` | `tidy-checks` (boolean array) – present only here. | +| `python-fix.yaml` | No extra inputs. | + +**Impact:** Callers need to remember workflow-specific additions. Recommendation: document each workflow's inputs in a shared reference (`.github/WORKFLOW_CALLS.md`). + +> **Note on de-duplication.** GitHub Actions does **not** support YAML anchors/aliases across files, and does not merge a `<<:`-injected map into `on.workflow_call.inputs`. There is no supported "import a shared inputs fragment" mechanism at the workflow level. De-duplicating input definitions therefore requires either (a) accepting per-workflow duplication (documented in one place), or (b) a generator/templating step that emits the workflow files from a single source. A raw `<<: *common_inputs` in a workflow's `inputs:` block will not work and must not be recommended. + +### 2.3 Consistency of `if:` gating + +The majority of jobs share an identical pattern: + +```yaml +if: > + github.event_name == 'workflow_dispatch' || + github.event_name == 'pull_request' || + github.event_name == 'push' || + github.event_name == 'schedule' || + github.event_name == 'workflow_call' || + ( + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(fromJSON('["OWNER", "COLLABORATOR", "MEMBER"]'), github.event.comment.author_association) && + startsWith(github.event.comment.body, format('@{0}bot ', github.event.repository.name)) + ) +``` + +*All workflows use the same block (modulo the `` part). No mismatches detected.* + +--- + +## 3. Fork-PR Compatibility + +> **Correction (supersedes the earlier draft).** The earlier draft claimed the +> `*-fix` workflows run on `pull_request` and therefore "break on forks." That +> is false. It also conflated two distinct write operations — *posting a +> comment* and *pushing a commit* — into one problem with one fix. Those are +> different problems with different solutions, and only one of them is solvable +> by an extra workflow invocation. This section is rewritten to reflect the +> workflows as actually wired. + +### 3.1 How the `*-fix` workflows are actually triggered + +Every `*-fix.yaml` workflow (`clang-format-fix`, `cmake-format-fix`, +`header-guards-fix`, `jsonnet-format-fix`, `markdown-fix`, `python-fix`, +`yaml-fix`) is triggered **only** by: + +* `issue_comment` (a `@phlexbot -fix` command on a PR), gated to + `OWNER`/`COLLABORATOR`/`MEMBER` author-association; +* `workflow_dispatch` (a maintainer manually running it); and +* `workflow_call` (invoked by `format-all.yaml`, which is itself + `issue_comment`-triggered under the same author-association gate). + +**None of them trigger on `pull_request`.** Consequently they never execute in a +fork's PR-event context, and the "read-only fork `GITHUB_TOKEN`" scenario the +earlier draft described **does not arise**. Every invocation runs in the +**base-repo context**, initiated by a trusted maintainer. There is no +fork-triggered code path to break. + +### 3.2 The two write operations, disentangled + +The `*-fix` workflows perform two different writes; the audit must treat them +separately because they have different credential requirements. + +| Operation | Where it happens | Credential requirement | +| ----------- | ------------------ | ------------------------ | +| **Post a PR comment** (status/patch instructions) | `action-handle-fix-commit`, `thollander/actions-comment-pull-request`, `action-complete-pr-comment` | Only needs `pull-requests: write`. In a base-repo context (or a `workflow_run` context) the plain `GITHUB_TOKEN` suffices — see `clang-tidy-report.yaml`, which comments with `github.token`, no PAT. | +| **Push the fix commit back to the PR branch** | `action-handle-fix-commit` → `git push origin HEAD:$PR_REF` | Needs a token that (a) can write to the target branch — possibly a **fork** branch when "Allow edits from maintainers" is enabled — and (b) **re-triggers downstream CI**. The default `GITHUB_TOKEN` cannot satisfy (b): commits pushed with `GITHUB_TOKEN` do **not** trigger further workflow runs (GitHub suppresses this to prevent recursion). This is the real, unavoidable reason `WORKFLOW_PAT` is present. | + +The fork case for the *commit* path is already handled gracefully: +`action-handle-fix-commit` pushes directly only when the PR is same-repo **or** +`maintainer_can_modify` is true; otherwise it uploads a `fix.patch` artifact and +comments patch-application instructions (see the action's `Create patch` / +`Comment with patch instructions` steps). It does **not** "fail on forks" — it +degrades cleanly. + +### 3.3 What an "extra workflow invocation" can and cannot solve + +The repo already uses the extra-invocation (`workflow_run`) pattern for +*comment* writes: `clang-tidy-report.yaml` and `codeql-comment.yaml`. +Instructively: + +* `clang-tidy-report.yaml` posts its comment with `github-token: ${{ github.token }}` — the plain `GITHUB_TOKEN` — because a `workflow_run` workflow runs in the base repo with `pull-requests: write`. **No PAT needed to comment.** +* `codeql-comment.yaml` uses `WORKFLOW_PAT` **only** to *download artifacts from another run in a private/internal repo* (see its inline comment on the `github-token` input); its `createComment`/`updateComment` calls could use `GITHUB_TOKEN`. The PAT there is an artifact-access requirement, not a commenting requirement. + +Therefore: + +* **Commenting → solvable by an extra invocation.** The comment-only steps of + the `*-fix` workflows *could* be moved to a `workflow_run`-triggered reporter + that comments with `GITHUB_TOKEN`, eliminating the PAT **for those steps**. + The benefit is modest, because these workflows already run in a base-repo + context where `GITHUB_TOKEN` also has `pull-requests: write`. +* **Committing → NOT solvable by an extra invocation.** A `workflow_run` + reporter changes *where* a write happens, not *what credential* a + CI-re-triggering `git push` requires. Pushing a fix commit that re-runs the + checks still needs a PAT or a GitHub App installation token. No amount of + extra workflow plumbing removes this requirement. + +### 3.4 Recommendations for fork handling + +* **Do not** refactor the `*-fix` workflows into a split "report" pattern on the + premise that they break on forks — they do not, and the split does not address + the operation (commit push) that actually needs the secret. +* **Optionally**, if minimizing PAT exposure in comment-only steps is desired, + move those specific comment steps behind the existing `workflow_run` pattern + and have them comment with `GITHUB_TOKEN`. Treat this as low-priority cleanup, + not a correctness fix. +* If the goal is to also remove the PAT from the **commit** path (e.g., to avoid + a long-lived user PAT entirely), the correct mechanism is a **GitHub App + installation token**, which both writes to the branch and re-triggers CI. A + second scoped user PAT (`WRITE_TOKEN`) does **not** achieve this — see §4. + +--- + +## 4. Permissions & `WORKFLOW_PAT` Usage + +| Workflow | Permissions block | `WORKFLOW_PAT` usage | Minimality assessment | +| ---------- | ------------------- | ---------------------- | ----------------------- | +| **Check-only** (clang-tidy-check, clang-format-check, yaml-check, markdown-check, python-check, codeql-analysis) | `contents: read`, `pull-requests: read` (plus `security-events: write` for codeql) | **None** | ✅ Minimal | +| **Fix / Report** (clang-tidy-fix, clang-tidy-report, coverage, markdown-fix, python-fix, clang-format-fix, jsonnet-format-fix, yaml-fix) | `contents: write` (often `issues`/`pull-requests: write`) | **Sees `secrets.WORKFLOW_PAT`** | ✅ Justified — the PAT is required to push CI-re-triggering fix commits (§4.1), not an over-permission artifact. Fine-grained-token provisioning (§4.2) is the appropriate minimization, if any. | +| **CI for actions** (actions/* CI workflows) | `contents: read`, `pull-requests: read`, `security-events: write` for codeql | Never uses PAT | ✅ Good | +| **Dependabot auto-merge** | `contents: write`, `pull-requests: write` | Uses `secrets.WORKFLOW_PAT` for merging | Acceptable (needs push). | + +### 4.1 Why `WORKFLOW_PAT` is required (corrected) + +The earlier draft asserted that `GITHUB_TOKEN` "would be sufficient if we adopt +the split-workflow model." That is incorrect for the operation that actually +drives the PAT requirement. + +* **The dominant reason is the commit push, not commenting.** The `*-fix` + workflows push fix commits back to the PR branch via `action-handle-fix-commit` + (`git push origin HEAD:$PR_REF`). Commits pushed with the default + `GITHUB_TOKEN` **do not trigger downstream workflow runs** — GitHub suppresses + this by design to prevent infinite CI recursion. Fix commits *must* re-trigger + the checks, so a credential other than `GITHUB_TOKEN` is required. A + `workflow_run` "report" split does not change this: the report workflow can + *comment* with `GITHUB_TOKEN`, but it cannot *push a re-triggering commit* with + it. +* **A secondary reason is cross-repo push to fork branches.** When a fork PR has + "Allow edits from maintainers" enabled, pushing to the contributor's branch + needs a token the base `GITHUB_TOKEN` does not carry. When that permission is + absent, the action already falls back to a patch artifact + instructions + (§3.2), so this is not a failure mode. +* **`codeql-comment.yaml`** uses `WORKFLOW_PAT` to download artifacts across + runs (required because `GITHUB_TOKEN` cannot access artifacts from other + private/internal repositories). This is **justified**. Its comment-posting + calls, by contrast, could use `GITHUB_TOKEN`. + +### 4.2 On a scoped `WRITE_TOKEN` secret (corrected) + +The earlier draft recommended introducing a second repo secret, `WRITE_TOKEN`, +scoped to `contents` + `pull-requests`. This recommendation is **withdrawn** for +the following reasons: + +* **A classic PAT cannot be narrowed to "contents + pull-requests only."** The + minimum classic-PAT scope that permits `git push` to a repo is `repo`, which + already encompasses contents and pull requests. There is no narrower classic + scope to gain. +* **If finer scoping is genuinely wanted, provision the *existing* + `WORKFLOW_PAT` as a fine-grained token** (repository-scoped, with + Contents: write and Pull requests: write permissions). This achieves the + minimization goal **without adding a second secret name**. +* **A second secret increases surface and onboarding friction.** Forks that want + `@phlexbot` auto-fixes must set `WORKFLOW_PAT` today; adding `WRITE_TOKEN` + would force them to set *two* secrets for no privilege gain. +* **To eliminate a long-lived user PAT entirely**, migrate to a **GitHub App + installation token** — it can both push (re-triggering CI) and comment, and is + short-lived and centrally revocable. This is the only option that also removes + the PAT from the *commit* path. + +For workflows that genuinely need `security-events` (codeql), keep the current +permission set and rely on `GITHUB_TOKEN` — no PAT is needed there. + +--- + +## 5. Action-Repo Workflow Parity + +### 5.1 Current state + +* Each `action/*` repository contains **CI workflows** (`ci.yaml`, `dependabot-auto-merge.yaml`, `guardrail-audit-alert.yaml`) that **lint**, **test**, and **security-scan** the action itself. +* **No copy of the *phlex* operational workflows** (e.g., `cmake-build.yaml`, `clang-tidy-check.yaml`) exists in the action repos – they are **not intended to run there**. + +### 5.2 Policy decision: reject "identical copies" of operational workflows + +> **Decision.** The "identical copies" framing is **rejected**. It is +> incompatible with §5.1 (the operational phlex workflows are intentionally +> absent from the action repos) and with how the two repo classes are actually +> used. The adopted policy is: *shared logic lives in reusable actions/workflows; +> each repository keeps only the workflow files it actually needs.* + +The two repo classes have genuinely different jobs: + +* The **phlex** operational workflows (`cmake-build`, `clang-tidy-check`, the + `*-fix` family) build and check the phlex C++/Python codebase. +* The **action** repos' CI (`ci.yaml`: actionlint, YAML/Markdown lint, CodeQL + `actions`) validates the *action definitions*. There is no C++ to build there; + copying `cmake-build.yaml` into, say, `action-handle-fix-commit` would be dead + weight. + +Consequently, "identical copies of operational workflows" is not a defect to +close — the absence is correct by design. + +The DRY mechanism GitHub actually provides is `workflow_call` / composite +actions, **which the repo already uses**. Consistency is already centralized +where it matters: `action-workflow-setup`, `action-handle-fix-commit`, +`action-collect-format-results`, etc. are single sources of truth consumed by +every workflow via SHA-pinned `uses:`. That is the supported, low-maintenance +form of "identical," and it is already in place. + +The one legitimate residual concern is **CI-policy drift** across the action +repos' near-identical `ci.yaml` files (permission blocks, version caps, +`guardrail-audit-alert.yaml` wording). This is real but small, and a file-copy +sync engine is a disproportionate — and failure-prone — response to it +(placeholder-substitution bugs, drift when a downstream copy is edited directly, +SHA-pin skew across N repos). + +### 5.3 If CI-policy consistency becomes a felt pain + +Address it with the supported reuse mechanism, not a copy script: + +1. **Preferred — reusable CI workflow.** Extract the shared action-repo CI into a + single reusable workflow (e.g., `action-ci.yaml` in `Framework-R-D/.github` or + a dedicated repo) that each action repo *calls*: + `uses: Framework-R-D//.github/workflows/action-ci.yaml@` with a + couple of inputs. One source of truth; no duplicated files to drift. +2. **Fallback — drift detection, not propagation.** A scheduled job that diffs + each action repo's `ci.yaml` against a canonical copy and **opens an issue** on + mismatch. Detect and flag; do not auto-overwrite downstream files. + +The file-sync script previously proposed here (`scripts/sync-action-ci.sh` with +`{{REPO}}` substitution) is **not recommended** and should not be built. + +--- + +## Prioritized Recommendations + +> **Superseded items.** Three recommendations from the earlier draft are +> **withdrawn**: (1) the P1 "refactor all `*-fix` workflows to the split pattern +> to fix fork breakage" — the `*-fix` workflows are not `pull_request`-triggered +> and do not break on forks (§3.1); (2) the P2 "introduce a scoped `WRITE_TOKEN` +> secret" — `WORKFLOW_PAT` is required for CI-re-triggering commit pushes and +> cannot be replaced by a split workflow or a narrower classic PAT (§3.3, §4); and +> (3) the P3 action-repo **file-sync script** — the "identical copies" policy is +> rejected (§5.2). The revised table below reflects the corrected findings. + +| Priority | Area | Recommendation | Rationale | +| ---------- | ------ | ---------------- | ----------- | +| **P2** | **Variable naming alignment** | Standardise all boolean inputs to **snake_case** (e.g., `skip-relevance-check`) and avoid lower-case env vars (`perfetto-heap-profile`). Use the same naming scheme for `workflow_dispatch` inputs. | Improves readability and reduces hidden bugs caused by case-sensitivity. Low-risk, self-contained. | +| **P2** | **Documentation** | Add a `WORKFLOW_CALLS.md` that lists all reusable workflow names, required inputs, and example invocations. Document the *actual* trigger model of the `*-fix` workflows (`issue_comment` / `workflow_dispatch` / `workflow_call`, base-repo context only) and the reason `WORKFLOW_PAT` is required (CI-re-triggering commit push). | Onboards contributors; prevents the same misdiagnosis recurring. | +| **P3** | **`workflow_dispatch` UX** | Ensure every `ref` input has a helpful description and a documented default (e.g., "empty → default branch"). Add a **`description`** field for extra inputs (`build-combinations`, `perfetto-…`). | Improves self-service usage from the UI; less confusion for ad-hoc runs. Independent, low-risk. | +| **P3** | **`workflow_call` input consistency** | Document the canonical base-input contract (`checkout-path`, `build-path`, `skip-relevance-check`, `ref`, `repo`, `pr-base-sha`, `pr-head-sha`) in `WORKFLOW_CALLS.md`. If de-duplication is pursued, use a **generator/templating** step — **not** a cross-file YAML anchor, which Actions does not support (§2.2). | Reduces drift without relying on an unsupported mechanism. | +| **P4** *(optional)* | **PAT minimisation** | If reducing token exposure is desired: (a) provision the existing `WORKFLOW_PAT` as a **fine-grained** repo-scoped token (Contents + Pull requests: write); and/or (b) migrate the commit-push path to a **GitHub App installation token**. Optionally move comment-only steps behind the existing `workflow_run` reporter so they comment with `GITHUB_TOKEN`. | Achieves least-privilege without a second secret; the GitHub App is the only route that also removes the PAT from the commit path. | +| **Closed** | **Action-repo CI parity** | Policy decision made (§5.2): "identical copies" of operational workflows is **rejected**; shared logic already lives in reusable actions consumed via SHA-pinned `uses:`. The file-sync script is **not** to be built. *If* CI-policy drift across action repos becomes a felt pain, use a **reusable `action-ci.yaml` workflow** (call, don't copy) or a **drift-detection** job that opens an issue on mismatch (§5.3). | Uses the supported reuse mechanism; avoids a copy engine's drift and SHA-skew failure modes. | +| **P4** | **Trigger regression guard** | Instead of a matrix job that "simulates" every trigger (`issue_comment`, `workflow_run`, `schedule` cannot be faithfully simulated from within a job), add a lightweight **lint/assertion** that checks the shared `if:` gating block is present and identical across workflows (e.g., a script in the existing `actionlint`/CI job). | Detects drift in the gating block without a mock that validates nothing real. | + +--- + +## Appendix – Quick Reference Tables + +### A.1 Workflow Input Summary (core) + +| Workflow | `checkout-path` | `build-path` | `skip-relevance-check` | `ref` | `repo` | `pr-base-sha` | `pr-head-sha` | +| ---------- | ----------------- | -------------- | -------- | ------- | -------- | ---------------- | --------------- | +| `cmake-build.yaml` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| `clang-tidy-check.yaml` | ✅ | ✅ (via setup) | ✅ | ✅ | ✅ | ✅ | ✅ | +| `clang-format-check.yaml` | ✅ | ✅ (via setup) | ✅ | ✅ | ✅ | ✅ | ✅ | +| `python-check.yaml` | ✅ | — | ✅ | ✅ | ✅ | ✅ | ✅ | +| `markdown-check.yaml` | ✅ | — | ✅ | ✅ | ✅ | ✅ | ✅ | +| `coverage.yaml` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | +| `yaml-check.yaml` | ✅ | — | ✅ | ✅ | ✅ | ✅ | ✅ | + +(“—” indicates the workflow never uses a build directory.) + +### A.2 Permission Matrix + +| Workflow | `contents` | `pull-requests` | `issues` | `security-events` | `secrets.WORKFLOW_PAT`? | +| ---------- | ------------ | ---------------- | ---------- | ------------------- | -------------------------- | +| `clang-tidy-check` | read | read | — | — | ✅ (only in `report` workflow) | +| `clang-tidy-fix` | write | write | — | — | ✅ (direct — required to push CI-re-triggering commits; §4.1) | +| `markdown-fix` | write | write | — | — | ✅ (direct — required to push CI-re-triggering commits; §4.1) | +| `codeql-analysis` | read | read | — | write | — | +| `codeql-comment` | read | read | write | — | ✅ (artifact download) | +| Action CI (`ci.yaml`) | read | read | — | write (codeql) | — | + +### A.3 Author-Association Gating Pattern + +The **check** workflows use the broad event list shown in §2.3. The **`*-fix`** +workflows use a *narrower* gate that does **not** include `pull_request` — this +is why they never run in a fork's PR-event context (§3.1): + +```yaml +if: > + inputs.ref != '' || + github.event_name == 'workflow_dispatch' || + (github.event_name == 'issue_comment' && + github.event.issue.pull_request && + contains(fromJSON('["OWNER","COLLABORATOR","MEMBER"]'), github.event.comment.author_association) && + startsWith(github.event.comment.body, format('@{0}bot -fix', github.event.repository.name))) +``` + +*The `inputs.ref != ''` clause is the `workflow_call` entry point (used by +`format-all.yaml`). The `*-report` workflows are `workflow_run`-triggered and +gate on `github.event.workflow_run.event == 'pull_request'` instead.* + +--- + +End of Report diff --git a/form/root_storage/root_rfield_read_container.cpp b/form/root_storage/root_rfield_read_container.cpp index c1f499639..d2ae2db5a 100644 --- a/form/root_storage/root_rfield_read_container.cpp +++ b/form/root_storage/root_rfield_read_container.cpp @@ -84,7 +84,7 @@ namespace form::detail::experimental { try { m_view = std::make_unique>(m_reader->GetView(col_name(), nullptr, type)); - } catch (const ROOT::RException& e) { + } catch (ROOT::RException const& e) { //RNTupleView will fail to create a field for fields written in streamer mode or for which type does not match the field's type on disk. Passing an empty string for type forces it to create the same type of field as the object on disk. Do this to handle streamer fields, then perform our own type check. m_view = std::make_unique>(m_reader->GetView(col_name(), nullptr, "")); @@ -111,7 +111,7 @@ namespace form::detail::experimental { m_view->BindRawPtr(buffer.get()); try { (*m_view)(id); - } catch (const ROOT::RException& e) { + } catch (ROOT::RException const& e) { throw std::runtime_error("ROOT_RField_Read_ContainerImp::read got a ROOT exception: " + std::string(e.what())); } diff --git a/plugins/python/python/phlex/_typing.py b/plugins/python/python/phlex/_typing.py index 7c08f9671..bbf8730b1 100644 --- a/plugins/python/python/phlex/_typing.py +++ b/plugins/python/python/phlex/_typing.py @@ -16,6 +16,7 @@ try: import numba.core.types as nb_types + has_numba = True except ImportError: has_numba = False @@ -48,21 +49,23 @@ } if has_numba: - _PY2CPP.update({ - nb_types.bool: "bool", - nb_types.int8: "int8_t", - nb_types.int16: "int16_t", - nb_types.int32: "int32_t", - nb_types.int64: "int64_t", - nb_types.uint8: "uint8_t", - nb_types.uint16: "uint16_t", - nb_types.uint32: "uint32_t", - nb_types.uint64: "uint64_t", - nb_types.Float: "float", - nb_types.float32: "float", - nb_types.double: "double", - nb_types.void: "None", - }) + _PY2CPP.update( + { + nb_types.bool: "bool", + nb_types.int8: "int8_t", + nb_types.int16: "int16_t", + nb_types.int32: "int32_t", + nb_types.int64: "int64_t", + nb_types.uint8: "uint8_t", + nb_types.uint16: "uint16_t", + nb_types.uint32: "uint32_t", + nb_types.uint64: "uint64_t", + nb_types.Float: "float", + nb_types.float32: "float", + nb_types.double: "double", + nb_types.void: "None", + } + ) # ctypes types that don't map cleanly to intN_t / uintN_t _CTYPES_SPECIAL: dict[type, str] = {} @@ -183,7 +186,7 @@ def normalize_type(tp: Any, globalns: Dict | None = None, localns: Dict | None = if origin is not None: args = typing.get_args(tp) - if origin in (np.ndarray, np.typing.NDArray): # numpy arrays + if origin in (np.ndarray, np.typing.NDArray): # numpy arrays dtype_args: tuple[Any, ...] = () # origin should point to the original type type, unless it is generic; it's # probably debatable what it should be for numpy.ndarray, but for versions diff --git a/scripts/check-gating-block.sh b/scripts/check-gating-block.sh new file mode 100755 index 000000000..c3ec18e87 --- /dev/null +++ b/scripts/check-gating-block.sh @@ -0,0 +1,169 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Verify that the shared if-gating blocks are present and consistent across +# GitHub Actions workflows. Exits 1 if any required phrase is missing. +# +# Two gating-block families are checked: +# +# CHECK-JOB pattern — used by the lint/check job in every *-check workflow: +# always() && ( +# github.event_name == 'workflow_dispatch' || +# inputs.skip-relevance-check || +# needs.setup.outputs.has_changes == 'true' +# ) +# +# FIX-SETUP pattern — used by the setup job in every *-fix workflow that +# exposes workflow_call (all fix workflows except clang-tidy-fix): +# inputs.ref != '' || +# github.event_name == 'workflow_dispatch' || +# (github.event_name == 'issue_comment' && +# ... author_association gate ...) + +WORKFLOWS_DIR="${1:-.github/workflows}" + +# Check workflows whose lint/check job uses the relevance-check if-gate. +CHECK_WORKFLOWS=( + "actionlint-check.yaml" + "clang-format-check.yaml" + "cmake-format-check.yaml" + "header-guards-check.yaml" + "jsonnet-format-check.yaml" + "markdown-check.yaml" + "python-check.yaml" + "yaml-check.yaml" +) + +# clang-tidy-check has an issue_comment-aware gating block on its setup job. +TIDY_CHECK_WORKFLOWS=( + "clang-tidy-check.yaml" +) + +# Fix workflows that expose workflow_call (use inputs.ref != '' as entry point). +# clang-tidy-fix is excluded: it has no workflow_call trigger. +FIX_WORKFLOWS=( + "clang-format-fix.yaml" + "cmake-format-fix.yaml" + "header-guards-fix.yaml" + "jsonnet-format-fix.yaml" + "markdown-fix.yaml" + "python-fix.yaml" + "yaml-fix.yaml" +) + +# cmake-build uses the full event-list gating block on its setup job. +BUILD_WORKFLOWS=( + "cmake-build.yaml" +) + +# Required phrases for the lint/check job in *-check workflows. +CHECK_JOB_PHRASES=( + "always()" + "github.event_name == 'workflow_dispatch'" + "inputs.skip-relevance-check" + "needs.setup.outputs.has_changes == 'true'" +) + +# Required phrases for the clang-tidy-check setup job. +TIDY_CHECK_PHRASES=( + "github.event_name == 'workflow_dispatch'" + "github.event_name == 'pull_request'" + "github.event_name == 'push'" + "github.event_name == 'issue_comment'" + "github.event.issue.pull_request" + "fromJSON('[\"OWNER\", \"COLLABORATOR\", \"MEMBER\"]')" + "github.event.comment.author_association" +) + +# Required phrases for fix workflow setup jobs (workflow_call-capable). +FIX_PHRASES=( + "inputs.ref != ''" + "github.event_name == 'workflow_dispatch'" + "github.event_name == 'issue_comment'" + "github.event.issue.pull_request" + "fromJSON('[\"OWNER\", \"COLLABORATOR\", \"MEMBER\"]')" + "github.event.comment.author_association" +) + +# Required phrases for cmake-build (full event-list gating on setup job). +BUILD_PHRASES=( + "github.event_name == 'workflow_dispatch'" + "github.event_name == 'pull_request'" + "github.event_name == 'push'" + "github.event_name == 'schedule'" + "github.event_name == 'workflow_call'" + "github.event_name == 'issue_comment'" + "github.event.issue.pull_request" + "fromJSON('[\"OWNER\", \"COLLABORATOR\", \"MEMBER\"]')" + "github.event.comment.author_association" +) + +pass=0 +fail=0 + +check_workflow() { + local file="$1" + local label="$2" + shift 2 + local phrases=("$@") + local missing=() + + if [[ ! -f "$file" ]]; then + echo "ERROR: $file not found" + fail=$((fail + 1)) + return + fi + + for phrase in "${phrases[@]}"; do + if ! grep -qF "$phrase" "$file"; then + missing+=("$phrase") + fi + done + + if [[ ${#missing[@]} -gt 0 ]]; then + echo "FAIL: $label" + for m in "${missing[@]}"; do + echo " missing: $m" + done + fail=$((fail + 1)) + else + echo "PASS: $label" + pass=$((pass + 1)) + fi +} + +echo "Check workflows (relevance-check gate on lint job)" +echo "===================================================" +for wf in "${CHECK_WORKFLOWS[@]}"; do + check_workflow "$WORKFLOWS_DIR/$wf" "$wf" "${CHECK_JOB_PHRASES[@]}" +done + +echo "" +echo "Clang-tidy check (issue_comment-aware gate on setup job)" +echo "=========================================================" +for wf in "${TIDY_CHECK_WORKFLOWS[@]}"; do + check_workflow "$WORKFLOWS_DIR/$wf" "$wf" "${TIDY_CHECK_PHRASES[@]}" +done + +echo "" +echo "Fix workflows (workflow_call-capable, inputs.ref gate)" +echo "=======================================================" +for wf in "${FIX_WORKFLOWS[@]}"; do + check_workflow "$WORKFLOWS_DIR/$wf" "$wf" "${FIX_PHRASES[@]}" +done + +echo "" +echo "Build workflow (full event-list gate on setup job)" +echo "===================================================" +for wf in "${BUILD_WORKFLOWS[@]}"; do + check_workflow "$WORKFLOWS_DIR/$wf" "$wf" "${BUILD_PHRASES[@]}" +done + +echo "" +echo "========================================" +echo "Results: $pass passed, $fail failed" +echo "========================================" + +if [[ $fail -gt 0 ]]; then + exit 1 +fi diff --git a/scripts/check_codeql_alerts.py b/scripts/check_codeql_alerts.py index b133213c0..5757d5770 100644 --- a/scripts/check_codeql_alerts.py +++ b/scripts/check_codeql_alerts.py @@ -710,6 +710,36 @@ def _format_section( return lines +# Sections with more items than this are wrapped in a collapsible
+# block so that PR comments with many alerts stay scannable by default. +_FOLD_THRESHOLD = 10 + + +def _render_section( + alerts: collections.abc.Sequence[Alert], + *, + max_results: int, + bullet_prefix: str, + label: str, +) -> list[str]: + """Formats a section's alert bullets, folding long lists behind
. + + Args: + alerts: The alerts to render. + max_results: The maximum number of results to include. + bullet_prefix: The emoji/text prefix for each bullet line. + label: The `` text shown for a folded section. + + Returns: + The formatted lines, wrapped in a `
` block when *alerts* + has more than `_FOLD_THRESHOLD` entries. + """ + lines = _format_section(alerts, max_results=max_results, bullet_prefix=bullet_prefix) + if len(alerts) <= _FOLD_THRESHOLD: + return lines + return ["
", f"{label}", "", *lines, "", "
"] + + def build_comment( *, new_alerts: collections.abc.Sequence[Alert], @@ -748,7 +778,14 @@ def _highest_severity(alerts: collections.abc.Sequence[Alert]) -> str | None: f"## ❌ {len(new_alerts)} new CodeQL alert" f"{'s' if len(new_alerts) != 1 else ''} (level ≥ {threshold}){sev_note}" ) - lines.extend(_format_section(new_alerts, max_results=max_results, bullet_prefix=":x:")) + lines.extend( + _render_section( + new_alerts, + max_results=max_results, + bullet_prefix=":x:", + label=f"{len(new_alerts)} new alert{'s' if len(new_alerts) != 1 else ''}", + ) + ) lines.append("") if fixed_alerts: @@ -757,8 +794,11 @@ def _highest_severity(alerts: collections.abc.Sequence[Alert]) -> str | None: f"{'s' if len(fixed_alerts) != 1 else ''} resolved since the previous run" ) lines.extend( - _format_section( - fixed_alerts, max_results=max_results, bullet_prefix=":white_check_mark:" + _render_section( + fixed_alerts, + max_results=max_results, + bullet_prefix=":white_check_mark:", + label=f"{len(fixed_alerts)} resolved alert{'s' if len(fixed_alerts) != 1 else ''}", ) ) lines.append("") @@ -1074,7 +1114,13 @@ def _build_multi_section_comment( "since the previous PR commit" ) lines.extend( - _format_section(api_comp.new_vs_prev, max_results=max_results, bullet_prefix=":x:") + _render_section( + api_comp.new_vs_prev, + max_results=max_results, + bullet_prefix=":x:", + label=f"{len(api_comp.new_vs_prev)} new alert" + f"{'s' if len(api_comp.new_vs_prev) != 1 else ''}", + ) ) lines.append("") if api_comp.fixed_vs_prev: @@ -1084,8 +1130,12 @@ def _build_multi_section_comment( "resolved since the previous PR commit" ) lines.extend( - _format_section( - api_comp.fixed_vs_prev, max_results=max_results, bullet_prefix=":white_check_mark:" + _render_section( + api_comp.fixed_vs_prev, + max_results=max_results, + bullet_prefix=":white_check_mark:", + label=f"{len(api_comp.fixed_vs_prev)} resolved alert" + f"{'s' if len(api_comp.fixed_vs_prev) != 1 else ''}", ) ) lines.append("") @@ -1097,7 +1147,13 @@ def _build_multi_section_comment( f"{'s' if len(api_comp.new_vs_base) != 1 else ''} since the branch point" ) lines.extend( - _format_section(api_comp.new_vs_base, max_results=max_results, bullet_prefix=":x:") + _render_section( + api_comp.new_vs_base, + max_results=max_results, + bullet_prefix=":x:", + label=f"{len(api_comp.new_vs_base)} new alert" + f"{'s' if len(api_comp.new_vs_base) != 1 else ''}", + ) ) lines.append("") if api_comp.fixed_vs_base: @@ -1106,8 +1162,12 @@ def _build_multi_section_comment( f"{'s' if len(api_comp.fixed_vs_base) != 1 else ''} resolved since the branch point" ) lines.extend( - _format_section( - api_comp.fixed_vs_base, max_results=max_results, bullet_prefix=":white_check_mark:" + _render_section( + api_comp.fixed_vs_base, + max_results=max_results, + bullet_prefix=":white_check_mark:", + label=f"{len(api_comp.fixed_vs_base)} resolved alert" + f"{'s' if len(api_comp.fixed_vs_base) != 1 else ''}", ) ) lines.append("") @@ -1127,7 +1187,13 @@ def _build_multi_section_comment( f"{'s' if len(api_comp.new_alerts) != 1 else ''} compared to main" ) lines.extend( - _format_section(api_comp.new_alerts, max_results=max_results, bullet_prefix=":x:") + _render_section( + api_comp.new_alerts, + max_results=max_results, + bullet_prefix=":x:", + label=f"{len(api_comp.new_alerts)} new alert" + f"{'s' if len(api_comp.new_alerts) != 1 else ''}", + ) ) lines.append("") if api_comp.fixed_alerts: @@ -1136,10 +1202,12 @@ def _build_multi_section_comment( f"{'s' if len(api_comp.fixed_alerts) != 1 else ''} resolved compared to main" ) lines.extend( - _format_section( + _render_section( api_comp.fixed_alerts, max_results=max_results, bullet_prefix=":white_check_mark:", + label=f"{len(api_comp.fixed_alerts)} resolved alert" + f"{'s' if len(api_comp.fixed_alerts) != 1 else ''}", ) ) lines.append("") diff --git a/scripts/test/test_check_codeql_alerts.py b/scripts/test/test_check_codeql_alerts.py index d39736a8e..8d2dd2ac7 100644 --- a/scripts/test/test_check_codeql_alerts.py +++ b/scripts/test/test_check_codeql_alerts.py @@ -531,6 +531,52 @@ def test_empty_list_returns_empty(self) -> None: assert M._format_section([], max_results=10, bullet_prefix=":x:") == [] +# --------------------------------------------------------------------------- +# _render_section +# --------------------------------------------------------------------------- + + +class TestRenderSection: + """Tests for _render_section().""" + + def _alert(self, n: int) -> M.Alert: + return M.Alert( + number=n, + html_url=f"https://example.com/{n}", + rule_id="py/r", + level="error", + message="msg", + location=f"src/app.py:{n}", + ) + + def test_short_list_not_folded(self) -> None: + """A list at or below the fold threshold is rendered unwrapped.""" + alerts = [self._alert(i) for i in range(M._FOLD_THRESHOLD)] + lines = M._render_section(alerts, max_results=100, bullet_prefix=":x:", label="N alerts") + assert "
" not in lines + assert len(lines) == M._FOLD_THRESHOLD + + def test_long_list_folded_with_details(self) -> None: + """A list above the fold threshold is wrapped in a
block.""" + alerts = [self._alert(i) for i in range(M._FOLD_THRESHOLD + 1)] + lines = M._render_section(alerts, max_results=100, bullet_prefix=":x:", label="11 alerts") + assert lines[0] == "
" + assert lines[1] == "11 alerts" + assert lines[-1] == "
" + # A blank line must separate the tag and the closing tag + # from the bullet list per GFM's requirement for a blank line after + # to render Markdown inside
. + assert lines[2] == "" + assert lines[-2] == "" + + def test_folded_output_contains_all_bullets(self) -> None: + """Folding must not drop any bullet lines.""" + alerts = [self._alert(i) for i in range(M._FOLD_THRESHOLD + 3)] + lines = M._render_section(alerts, max_results=100, bullet_prefix=":x:", label="label") + bullet_lines = [ln for ln in lines if ln.startswith("- :x:")] + assert len(bullet_lines) == len(alerts) + + # --------------------------------------------------------------------------- # build_comment # --------------------------------------------------------------------------- @@ -665,6 +711,31 @@ def test_highest_severity_shown(self) -> None: ) assert "Error" in body # highest severity title + def test_long_new_alert_list_is_folded(self) -> None: + """A new-alerts section above the fold threshold is collapsed.""" + alerts = [self._alert() for _ in range(M._FOLD_THRESHOLD + 1)] + body = M.build_comment( + new_alerts=alerts, + fixed_alerts=[], + repo=None, + max_results=100, + threshold="warning", + ) + assert "
" in body + assert "" in body + + def test_short_new_alert_list_is_not_folded(self) -> None: + """A new-alerts section at or below the fold threshold is not collapsed.""" + alerts = [self._alert() for _ in range(M._FOLD_THRESHOLD)] + body = M.build_comment( + new_alerts=alerts, + fixed_alerts=[], + repo=None, + max_results=100, + threshold="warning", + ) + assert "
" not in body + # --------------------------------------------------------------------------- # _to_alert_api @@ -1146,6 +1217,21 @@ def test_new_vs_base_rendered(self) -> None: assert "since the branch point" in body assert "abc1234" in body + def test_long_new_vs_base_list_is_folded(self) -> None: + """A new_vs_base section above the fold threshold is collapsed.""" + alerts = [self._alert(number=i) for i in range(M._FOLD_THRESHOLD + 1)] + comp = self._comp(new_vs_base=alerts, base_sha="abc1234") + body = M._build_multi_section_comment(comp, max_results=100) + assert "
" in body + assert "" in body + + def test_short_new_vs_base_list_is_not_folded(self) -> None: + """A new_vs_base section at or below the fold threshold is not collapsed.""" + alerts = [self._alert(number=i) for i in range(M._FOLD_THRESHOLD)] + comp = self._comp(new_vs_base=alerts, base_sha="abc1234") + body = M._build_multi_section_comment(comp, max_results=100) + assert "
" not in body + def test_fixed_vs_prev_rendered(self) -> None: """Fixed vs prev rendered.""" comp = self._comp( diff --git a/test/python/adder.py b/test/python/adder.py index 4205a5736..3760a7972 100644 --- a/test/python/adder.py +++ b/test/python/adder.py @@ -57,14 +57,12 @@ def PHLEX_REGISTER_ALGORITHMS(m, config): int_adder = Variant(add, {"i": int, "j": int, "return": int}, "iadd") try: - # intentional failure to check error path of missing output suffix - m.transform(int_adder, input_family=config["input"]) + # intentional failure to check error path of missing output suffix + m.transform(int_adder, input_family=config["input"]) except TypeError as e: - assert "should have an output suffix" in str(e) + assert "should have an output suffix" in str(e) else: - raise AssertionError( - "m.transform() should reject registrations without an output suffix" - ) + raise AssertionError("m.transform() should reject registrations without an output suffix") # functional transform registration m.transform(int_adder, input_family=config["input"], output_product_suffixes=config["output"]) diff --git a/test/python/jited.py b/test/python/jited.py index 436bda744..af6a91606 100644 --- a/test/python/jited.py +++ b/test/python/jited.py @@ -15,8 +15,8 @@ ("u1", "u2", np.uint32, 1), ("l1", "l2", np.int64, 1), ("ul1", "ul2", np.uint64, 100), - ("f1", "f2", np.float32, 1.), - ("d1", "d2", np.float64, 1.), + ("f1", "f2", np.float32, 1.0), + ("d1", "d2", np.float64, 1.0), ) @@ -40,24 +40,28 @@ def PHLEX_REGISTER_ALGORITHMS(m, config): def new_o(x): def o(y): assert y == x + return o for arg0, arg1, t, res in specs: tn = t.__name__ f_a = numba.cfunc(f"{tn}({tn}, {tn})", nogil=True, nopython=True, cache=True)(add) - m.transform(f_a, - name="add_"+tn, - input_family=[{"creator": "input", "layer": "event", "suffix": arg0}, - {"creator": "input", "layer": "event", "suffix": arg1}], - output_product_suffixes=["sum_"+tn], - concurrency=4) + m.transform( + f_a, + name="add_" + tn, + input_family=[ + {"creator": "input", "layer": "event", "suffix": arg0}, + {"creator": "input", "layer": "event", "suffix": arg1}, + ], + output_product_suffixes=["sum_" + tn], + concurrency=4, + ) f_o = numba.cfunc(f"void({tn})", nogil=True, nopython=True, cache=True)(new_o(res)) - m.observe(f_o, - name="obs_"+tn, - input_family=[ - {"creator": "add_" + tn, "layer": "event", "suffix": "sum_"+tn} - ], - concurrency=4) - + m.observe( + f_o, + name="obs_" + tn, + input_family=[{"creator": "add_" + tn, "layer": "event", "suffix": "sum_" + tn}], + concurrency=4, + )