Repository navigation
Conversation
Both procedures apply at a known moment: before editing CI configuration or a script CI runs, and before calling a change done. AGENTS.md loaded them into every session. - ci-script-change: find every caller, enumerate the script's behavior branches from its history, prove a removal with a repo-wide grep, and check each calling job. Moved from "CI / Automation Script Changes", plus a pointer to the hermetic script test suites. - verify-profiler-change: which builds and tests each kind of change needs, by area. Moved from the first Development Guidelines bullet and extended with rows for JVM-specific, libc-sensitive, build-logic, script and doc changes, plus rules for flaky and blocked checks. Commands stay in doc/building/TestingGuide.md. AGENTS.md keeps "never claim success until all affected tests pass" and one-line pointers to both skills. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- ci-script-change and its AGENTS.md trigger cover deleting any tracked file, as the moved rule did, not only scripts. Callers are searched repo-wide, including Gradle files and hooks, and test suites are found with git ls-files instead of a partial list. - verify-profiler-change adds gtestTsan for signal-handler and atomic changes, says the containers have no Zing JDK (report blocked), covers .github and .gitlab scripts, states that the Pi cases need Node.js 22.6, and that running the check scripts directly is not qualityChecks. It drops the bug-fix test bullet that AGENTS.md already states. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Scan-Build Report
Bug Summary
Filter Results by FileReports
|
|||||||||||||||||||||||||||||||||||||||||||||||||
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e441e87204
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The skill told an agent without a local Zing to report the check as blocked and name the CI job that covers it. No such job exists: every Zing entry in CI is commented out because the Zing downloads are gone (.github/workflows/cache_java.yml). The skill now says so, and the general blocked-check rule allows "no CI job covers it". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
The verify-profiler-change SKILL.md instructs contributors to name a CI job covering Zing-specific changes, but Zing is excluded from all active matrices in test_workflow.yml and its downloads are disabled in cache_java.yml, so no such job exists and the procedure cannot be followed. Both the JVM row and the blocked-check rule need to be updated to explicitly allow reporting that no CI coverage is available for Zing.
🤖 Bits Code Review · Commit e441e87
…16542) Review follow-ups to the Zing fix: - The JVM row also covers shared code that branches on VM::isHotspot(), VM::isOpenJ9() or VM::isZing(), where most Zing behavior lives. - Instead of stating today's CI state, the row tells the agent to check the JDK matrix in .github/workflows/test_workflow.yml for a -zing entry before claiming coverage. The statement cannot go stale when Zing is re-enabled. - The blocked-check rule says where to look for a covering CI job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The previous commit told agents to search test_workflow.yml for a -zing entry. The file still contains -zing in commented-out matrices and in conditionals, so the search reports coverage that does not exist. The row now states the fact and cites doc/architecture/JvmSupportLayer.md, which records that Zing is disabled in the CI JDK matrix. The search for a covering job includes the .gitlab/ files that .gitlab-ci.yml includes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI Test ResultsRun: #42838093658 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-11-10 15:14:35 UTC |
There was a problem hiding this comment.
More details
The extracted procedures preserve caller tracing and behavior-preservation safeguards. The verification guidance explicitly marks unavailable Zing checks as blocked with no CI coverage, avoiding an unsupported claim of validation.
🤖 Bits Code Review · Commit 34545bb
|
What does this PR do?:
Moves two procedures from
AGENTS.mdinto agent skills:.agents/skills/ci-script-change/: before changing CI configuration or a script that CI, Gradle or a hook runs, and before deleting a tracked file or removing a default entry:.agents/skills/verify-profiler-change/: which builds and tests each kind of change needs before it is called done, and how to report flaky and blocked checks. Commands stay indoc/building/TestingGuide.md.AGENTS.mdkeeps "never claim success until all affected tests pass" and a one-line pointer to each skill. It shrinks from 544 to 534 lines.Stacked on #959, which adds the
check-skills.pygate that these skills pass.Motivation:
Both procedures apply at a known moment: editing CI, or claiming a change is done.
AGENTS.mdloaded them into every session. Step 2 of PROF-16466; design on PROF-16542 (decision D3 narrowsverify-profiler-changeto which tests to run).Additional Notes:
verify-profiler-changeadds rows for JVM-specific, libc-sensitive, build-logic, script and doc changes;gtestTsanfor signal-handler and atomic changes;blockedwith no CI coverage. Codex and Bits found that the first version asked for a covering CI job that does not exist. A later attempt to search the CI matrix gave false matches, so the skill states the fact and citesdoc/architecture/JvmSupportLayer.md. When Zing returns to CI, both need updating together;VM::isHotspot(),VM::isOpenJ9()orVM::isZing()./code-review(medium) on the first commit found 10 issues: narrowed triggers, an incomplete test-suite list, a missing TSan run, the Zing gap, a wrong Node.js version and one duplicated rule. The second commit fixes all 10./code-review(low) on the fix found nothing. No Sphinx run, because the PR changes only Markdown.How to test the change?:
python3 utils/check-skills.pybash utils/tests/test_quality_check_scripts.shprepare_reports.sh" loadsci-script-change; "what must pass after changing a signal-handler flag load" loadsverify-profiler-changemainvs. this branchgit log, search its callers and runbash -n; onmain, neither read the history. Both verify answers name ASan, TSan, the aarch64 container, the Gradle wrapper andblocked; onmain, Pi named neither TSan norblocked, and Claude Code named all fiveFor Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.🤖 Generated with Claude Code