maintenance/workflows audit#687
Open
greenc-FNAL wants to merge 7 commits into
Open
Conversation
Introduce a comprehensive audit of GitHub Actions workflows across the `phlex` monorepo and sibling `action/*` repositories. The report analyzes: - Structural consistency and naming of `workflow_call` inputs. - Trigger correctness and `if:` gating logic. - Fork-PR compatibility, highlighting failures in `*-fix` workflows. - Permission minimality and `WORKFLOW_PAT` usage. - CI parity between the primary repo and action repositories. It concludes with prioritized recommendations to improve security (least-privilege tokens), reliability (split-workflow pattern for forks), and maintainability (centralized CI sync script).
Create a new developer guide explaining the workflow for producing and running structured execution plans using the Kilo agent system. The documentation covers: - Using the `plan` sub-agent to generate JSON-based execution plans. - The structure of a plan, including idempotency, step definitions, and model selection. - Executing plans via the `orchestrate` agent. - Model selection policies and verification using `prek`.
Switch `devcontainer.json` to use `docker-compose.yml` for service definition, volume mounts, and environment variables to improve configurability and consistency. Key changes include: - Update `Dockerfile` to use a pinned base image (`2026-06-24`), install additional utilities (`jq`, `tree`, `yq`), and upgrade Node.js to 26.x. - Replace `kiro-cli` with `@kilocode/cli` installed via npm. - Refactor `KILO_CONFIG_CONTENT` handling: `ensure-repos.sh` now generates a `.phlex-kilo.env` file on the host which is loaded by Docker Compose. This ensures the VS Code plugin receives the correct configuration (including the `host.docker.internal` relay) at startup, replacing the previous `.bashrc` modification in `post-create.sh`. - Remove Copilot-related extensions and settings from the workspace and devcontainer configuration.
Update the guide to reflect recent changes in the Kilo CLI and agent behavior: * Update `kilo run` usage to use positional arguments for prompts instead of `--prompt` and `--description` flags. * Rename the `orchestrate` agent to `execute`. * Document in-flight re-planning, including the `on_error: replan` mechanism and the `mode: all` configuration for the `plan` agent. * Clarify that only `fnal-litellm` provider models are permitted and update model examples and defaults.
Rename the `plan` agent to `genplan` to avoid conflicts with the Kilo CLI's reserved native "Plan mode". Expand the guide with detailed sections on: * Model selection policy: Justification for using `gpt-oss-120b` for planning and `claude-sonnet-4-6` for orchestration. * Resuming interrupted executions: Guidance on using `--continue`, `--session`, and `--fork` based on plan idempotency. * Invocation modes: Comparison between non-interactive (headless) and interactive (`-i`) modes.
- Create docs/dev/WORKFLOW_CALLS.md documenting reusable workflow interfaces, inputs, and trigger models. - Add scripts/check-gating-block.sh and a corresponding CI job in actionlint-check.yaml to enforce consistency of if-gating blocks. - Standardize boolean inputs in cmake-build.yaml to snake_case (perfetto_heap_profile, perfetto_cpu_profile). - Reduce WORKFLOW_PAT usage in codeql-comment.yaml by switching to github.token for PR commenting. - Update workflow-audit-report.md with corrected analysis on fork handling and token requirements, and add workflow-audit-implementation-plan.json. - Apply pre-commit style fixes to form/ and test/python/.
…ocks Add _render_section() function to wrap alert sections exceeding _FOLD_THRESHOLD (10 items) in collapsible <details> blocks, improving readability of PR comments with many alerts. Short lists remain unwrapped. Replace all _format_section() calls with _render_section() in build_comment() and _build_multi_section_comment(), passing appropriate summary labels for each section (new alerts, resolved alerts, etc.). Add comprehensive test coverage for _render_section() including: - Short lists not folded - Long lists wrapped with proper <details>/<summary> structure - Blank line separation for GFM Markdown rendering inside <details> - All bullets preserved when folded Add integration tests verifying folding behavior in build_comment() and _build_multi_section_comment() workflows.
greenc-FNAL
requested review from
aolivier23,
knoepfel,
marcpaterno and
sabasehrish
as code owners
Contributor
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+132
to
+140
| - 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates a “workflow audit” effort by documenting current GitHub Actions patterns (and recommended policies), adding CI guardrails for workflow gating blocks, improving CodeQL PR-comment readability by folding long alert lists, and updating the devcontainer implementation to a Docker Compose-based setup with refreshed tooling.
Changes:
- Add workflow audit documentation (report, implementation plan JSON, and
WORKFLOW_CALLS.md) plus a Kilo plan generation/execution guide. - Add a gating-block verification script and wire it into
actionlint-checkas an additional CI job. - Improve operational UX: fold long CodeQL alert sections behind
<details>, standardize Perfetto workflow_dispatch input names, and migrate devcontainer to Docker Compose with updated toolchain.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
test/python/jited.py |
Minor numeric-literal and formatting normalization for tests. |
test/python/adder.py |
Formatting/indent consistency in an error-path test. |
scripts/test/test_check_codeql_alerts.py |
Adds unit coverage for the new _render_section folding behavior. |
scripts/check-gating-block.sh |
New script to assert required gating phrases exist in workflow files. |
scripts/check_codeql_alerts.py |
Implements folding of long CodeQL alert sections via <details> blocks. |
plugins/python/python/phlex/_typing.py |
Formatting-only adjustments. |
form/root_storage/root_rfield_read_container.cpp |
Minor C++ style tweak (east-const style in catch). |
docs/dev/workflow-audit-report.md |
Adds an audit report capturing workflow semantics and recommendations. |
docs/dev/workflow-audit-implementation-plan.json |
Adds a machine-readable implementation plan for audit recommendations. |
docs/dev/WORKFLOW_CALLS.md |
Adds a reusable-workflow input/usage reference document. |
docs/dev/generating-executing-kilo-plans.md |
Adds a guide for generating/executing Kilo plans (genplan/execute). |
.github/workflows/codeql-comment.yaml |
Uses github.token for comment write actions (PAT retained for artifact download). |
.github/workflows/cmake-build.yaml |
Renames perfetto workflow_dispatch inputs to snake_case and updates references. |
.github/workflows/actionlint-check.yaml |
Adds gating_check job to run the gating-block verification script. |
.devcontainer/post-create.sh |
Removes KILO_CONFIG_CONTENT shell wiring and kiro-cli install. |
.devcontainer/ensure-repos.sh |
Generates .phlex-kilo.env for Docker Compose/devcontainer Kilo config injection. |
.devcontainer/Dockerfile |
Pins base image tag and installs additional dev utilities + Node/Kilo CLI. |
.devcontainer/docker-compose.yml |
New Compose definition for the devcontainer service and mounts. |
.devcontainer/devcontainer.json |
Switches from Dockerfile-only to Docker Compose-based devcontainer config. |
.devcontainer/codespace.code-workspace |
Removes Copilot Chat recommendation from the workspace file. |
Comment on lines
+539
to
+540
| class TestRenderSection: | ||
| """Tests for TestRenderSection.""" |
Comment on lines
+4
to
+5
| # Verify that the shared if-gating blocks are present and consistent across | ||
| # GitHub Actions workflows. Exits 1 if any required phrase is missing. |
Comment on lines
95
to
+100
| 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 |
Comment on lines
+108
to
+111
| 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 <name>', github.event.repository.name)) |
Comment on lines
+189
to
+199
| # 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. | ||
| 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}'" > "${HOME}/.phlex-kilo.env" | ||
| fi |
Contributor
Author
|
21 fixed, 1 new since branch point (cf6781a) ❌ 1 new CodeQL alert since the previous PR commit
✅ 21 CodeQL alerts resolved since the previous PR commit21 resolved alerts
❌ 1 new CodeQL alert since the branch point
✅ 21 CodeQL alerts resolved since the branch point21 resolved alerts
Review the full CodeQL report for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #687 +/- ##
==========================================
+ Coverage 83.43% 83.63% +0.20%
==========================================
Files 174 174
Lines 6820 6913 +93
Branches 837 848 +11
==========================================
+ Hits 5690 5782 +92
+ Misses 853 851 -2
- Partials 277 280 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Contributor
Clang-Tidy Check ResultsFound 5762 issue(s); none are newly introduced by this patch. All issues by check:
See inline comments for details. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
blocks