Repository navigation
CI: stop Draft runner flood, scope main and firmware pushes, package on demand - #158
Conversation
masarray
left a comment
There was a problem hiding this comment.
CI policy review (2026-10-15) — audited exact PR head 90d54a930d63 against current main@35eda54.
Confirmed: 26 changed paths are limited to 24 existing workflows, scripts/check_ci_trigger_policy.py, and docs/CI_WORKFLOW_ROUTING.md; no SMV/PTP firmware or native GUI runtime code changed. The CI Routing Policy job passed at run 38641929343. Raw GitHub PR metadata currently reports mergeable_state=clean, though main has advanced by one unrelated Workbench-only commit.
Integration cautions:
- Skipped Draft jobs are intentionally not compiler/firmware/installer verification. Transitioning this 24-workflow PR to Ready will launch the path-matched heavy regression wave; merging will also result in
mainpath-matched jobs because each modified workflow includes its own YAML in its trigger paths. Do not call these equivalent to a single cheap CI run. - M3 Draft PR #157 / stacked #166 contains
phase-guard-checkpoint, a special opt-in Draft job guarded by a PR-body marker, not the usual!draftcondition. New routing script currently assertslen(guarded)==len(names)for every PTP workflow job. When synchronizing the new CI policy into M3, retain the bounded checkpoint and explicitly teach the routing checker about only this allowlisted opt-in condition, or migrate it to a controlledworkflow_dispatchinput. Blindly replacing M3's PTP YAML with this PR's variant would remove its verified targeted checkpoint. - Keep #158 as Draft until one deliberate integration wave is scheduled; do not rush merge just to remove the Draft badge. No new CI run is requested by this review.
This review is informational; not approval of all downstream firmware/GUI matrix results.
Codex Review SummaryThis 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: 74ad858f60
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - 'include/ariec61850/asn1/**' | ||
| - 'src/asn1/**' | ||
| - 'tests/fixtures/scl/**' | ||
| - '.github/workflows/brcb-hard-profile-ci.yml' |
There was a problem hiding this comment.
Include OSI and ACSE dependencies in the BRCB push scope
The new main path filter omits src/osi/**, include/ariec61850/osi/**, src/acse/**, and their headers even though embedded/brcb_hard_profile/CMakeLists.txt directly compiles those sources into the lifecycle, connection, and negotiated-limits smoke targets. A merged OSI/ACSE change therefore no longer runs this specialized -fno-exceptions BRCB gate, so failures unique to the hard profile can reach main despite the workflow previously running on every main push. Add every directly compiled dependency domain to this scope.
AGENTS.md reference: AGENTS.md:L256-L261
Useful? React with 👍 / 👎.
| - 'tests/fixtures/scl/**' | ||
| - '.github/workflows/brcb-hard-profile-ci.yml' | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] |
There was a problem hiding this comment.
Handle converted-to-draft events to cancel active jobs
When a Ready PR is switched back to Draft while one of these expensive runs is active, GitHub emits the converted_to_draft pull-request activity, but the new types list excludes it. No replacement workflow run is created, so the PR-scoped cancel-in-progress concurrency policy cannot cancel the already-running matrix and the jobs continue consuming runners despite the PR now being Draft. Include converted_to_draft; its jobs will be skipped by the draft guard while the workflow-level concurrency cancels the older run.
AGENTS.md reference: AGENTS.md:L15-L16
Useful? React with 👍 / 👎.
…mediates M2F: constrain native Workbench GUI MMS string/hex/array serialization at source before oversized intermediates can be created. Preserve scalar canonical codec semantics and original runtime MMS values, respect UTF-8 boundaries, cap nested recursion, and add large-payload native regressions (2MB strings, 1MB octets, nested payloads, parity and zero budget). All 51 exact-head Qt CI steps passed. Compatible with newly merged CI optimization PR #158.
CI routing hotfix — synchronized with main (2026-10-15)
Actual implementation: one combined/atomic merge commit
90d54a930d638500443d4485c036257e16164474updates 24 existing workflow files plus the CI routing contract script and documentation. It incorporates currentmainrather than reverting unrelated GUI/IED work.Changes that reduce running jobs, not just hide checks
mainwith matching paths. Preserve explicitworkflow_dispatchfor on-demand firmware artifacts andworkflow_callused by release.v*/ manual only. No Windows/firmware release package on normal protocol PRs.ready_for_reviewand later Ready-PR synchronize still run relevant GCC/Clang/MSVC, embedded, ESP-IDF, Qt, PTP and security regression. No tests deleted or silently marked PASS.main. Normal CI on unrelated commits no longer fans out to all domains.scripts/check_ci_trigger_policy.pyenforces all these invariants;docs/CI_WORKFLOW_ROUTING.mddocuments Draft/checkpoint/Ready/main/release lifecycles.Evidence
Merge safety and rollout
main. GitHub reports mergeable/clean and comparison is current. This avoids inadvertently starting a double wave of heavy Ready-PR and main-merge jobs.mainwill use the new scopes. Do not infer that skipped jobs are validated or that CI-run count (skipped metadata records) is zero.