Skip to content

Reuse unchanged-input classification for merge groups - #831

Merged
rmcdaniel merged 6 commits into
mainfrom
fix/325-merge-group-fixture-inputs
Oct 28, 2026
Merged

rmcdaniel merged 6 commits into
mainfrom
fix/325-merge-group-fixture-inputs

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

PHP-only merge groups repeat all 43 Shared Server fixture jobs even when none of their inputs changed. #828's shared merge-group qualification took about 22 minutes alongside the separate PHP checks. Reuse the input classifier for a merge group whose checked-out head matches the event, whose base is an ancestor and verified current public-origin main, and whose complete range changes no shared input. Unavailable/moving main, shallow or mismatched history and unclassifiable paths retain full qualification. The remote read has a 30-second bound and suppresses remote error details.

Before making merge groups selective, repair gaps in the inherited input list. Native tests consume config/dw-contract.php, activity metadata, codec, history-budget and upgrade fixtures outside rust/**; the harness also imports proxy-target.mjs and CI helpers outside the old parity prefix. Retain every existing pattern and broaden coverage for those inputs, conformance/CI helpers, shared contract/observation modules and parent Cargo/checkout controls. A source inventory guard checks the literal native inputs and relative parity imports and literal workflow helper invocations. Explicit PR and merge-group cases cover the configuration contract.

All fixture jobs, matrices, commands, the required terminal result, permissions and resource limits remain intact. PR classification uses the expanded canonical path list. Push/manual classification remains full. The workflow differs only by 13 added input patterns and three merge-group identity fields.

Validation with the same 78 classifier/aggregate/partition tests:

Variant Passing Expected failures
Current implementation and complete input list 78 0
Original classifier/workflow 72 6
Current classifier with inherited input patterns and event wiring 62 16

Real Git fixtures cover both queued-predecessor base layouts, malformed/stale identities, non-ancestor and shallow bases, moved/unavailable main, invalid file names and suppressed remote details. Pinned actionlint passes. The workflow comparison confirms the rest of the file is unchanged. No Rust build or test ran locally.

Combined head 0ba815f02327ca1c46a7f520c7e783975132a5a1 integrates current main 4ccbfb5b, including #833/#834. The three changed source blobs are identical to the reviewed and locally qualified implementation. All 78 contract cases pass on the combined source; the unchanged workflow retains its pinned-actionlint proof. Integration/source review.

Normal head qualification is complete: 56 successful checks and one conditional skip, including all 43 jobs in Shared fixtures and successful PHP qualification. Sonar reports zero open issues and an OK quality gate. Normal merge-group qualification is now running at 1bc585205d18cecb9fd2b05ca02b3770eb9d5828: Shared fixtures, PHP; boundary passed. This group changes the classifier/workflow itself and requires full qualification.

Refs #830 and #325. Keep #830 open through an actual unchanged-input hosted merge-group observation after merge, including selected jobs, timings and limits. No hosted speedup or sustained-throughput improvement is claimed yet.

@rmcdaniel

rmcdaniel commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

Resolved at e3026ef: independent follow-up verification receipt. The original c834c75 evidence below is preserved.

Cloud review finding at exact head c834c75ecef8aa61e3741ad6989fc73c8419e96a: the reused input list omits a real native-test input, config/dw-contract.php.

The native configuration audit reads that file directly, and its contract test requires every entry to have a disposition. This is the same dependency whose valid extension caused the earlier #812 native-audit failure. The old unconditional merge-group run caught that drift even though the PR path list omitted the file.

Independent offline Node 22.23.3 review: the existing aggregate/classifier module passes 56/56 tests, with no failures or skips. An additional isolated real-Git fixture changes only config/dw-contract.php, adding a contract entry from 165 to 166. With the PR identity valid, and separately with the merge-group head/base/ancestry and origin-main identity valid, the exact classifier reports:

changed_files = ["config/dw-contract.php"]
matched_inputs = []
pull_request = run=false
merge_group = run=false

The merge-group fast path would therefore omit required native-audit coverage for that input. Please add config/dw-contract.php to the shared input declaration and add PR and merge-group regression cases changing only this contract expecting run=true. Keep the current identity and fallback controls. This finding concerns input completeness; I did not execute Rust or claim a hosted fast-path observation. The reviewed product source was unchanged.

@rmcdaniel

Copy link
Copy Markdown
Member Author

The input audit found that the inherited PR/push list omitted files consumed by native tests: config/dw-contract.php, activity metadata, codec, history-budget and upgrade fixtures. A transitive parity import (proxy-target.mjs) and CI helpers also fell outside the old prefix. Merge groups previously always ran the matrix, so making them selective required closing those gaps first.

Updated #831 to preserve every existing pattern and add the external inputs, conformance/CI helpers, shared contract/support modules and parent Cargo/checkout controls. The new inventory regression checks literal native repository inputs and relative parity imports. All fixture jobs/matrices/commands, permissions, resource limits and the terminal result remain unchanged.

All 76 current classifier/aggregate/partition tests pass. The same suite with the original classifier/workflow has four expected failures and 72 passing controls. Keeping the new classifier/event wiring but restoring the inherited input patterns fails 14 controls, with 62 passing. This establishes why the coverage expansion is required. Pinned actionlint passes; the workflow is otherwise byte-identical after removing only the 13 new input patterns and three event fields.

Current combined head c2755e7924804f01bb6e18adbd583782622b6c97 includes accepted main abfd6114. The three CI files are unchanged from the tested implementation; no Rust runtime or frozen expectation file was edited. Normal current-head hosted checks and an actual unchanged-input hosted merge-group observation remain required before acceptance or a timing claim.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Addressed the input-coverage finding in review 6460669071 and the native-owner dependency audit at current head e3026efd13225b52e69363ed99f290a3a6ce139c.

config/dw-contract.php is declared and tested explicitly for both PR and merge-group classification. The canonical list also covers the four external native fixture groups, CI/conformance helpers, shared contract/observation modules and parent Cargo/checkout controls. Mechanical guards check literal native includes/manifest-relative inputs, relative parity imports and literal workflow helper invocations.

All 78 current tests pass. The same suite with the original classifier/workflow has six expected failures and 72 passing controls. Restoring the inherited input patterns while keeping the new classifier/event wiring fails 16 controls, with 62 passing. Existing jobs, matrices, commands, permissions and limits are unchanged. Current-head hosted qualification is running; this does not claim an independent reviewer has retested the new head or that hosted fast-path timing has been observed.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Reviewed exact head e3026efd13225b52e69363ed99f290a3a6ce139c, including the input-coverage correction requested in 6460669071.

The merge-group decision checks the event identity and complete base-to-head range. It can skip only after verifying that the base is current main; an unmerged predecessor, shallow/missing history, unsupported path syntax or unavailable/moving main retains full qualification. The remote check is read-only, bounded to 30 seconds and does not reproduce remote details on failure. The expanded canonical input list covers the audited external native configuration/fixtures and transitive harness helpers. Existing patterns, jobs, matrices, test commands, permissions, limits and terminal failure handling remain intact.

All 78 local aggregate/classifier/partition cases and pinned actionlint pass. Original-source controls fail six cases; restoring the inherited input list fails 16. Current-head Sonar analysis and its quality gate pass with zero new open issues. Normal hosted native/PHP/CodeQL qualification is still running and must pass through the repository's required checks and merge queue. Actual unchanged-input hosted merge-group selection/timing remains acceptance work in #830; no measured speedup is claimed yet.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Cloud follow-up review at e3026efd13225b52e69363ed99f290a3a6ce139c: the input-coverage blocker in 6460669071 is resolved.

  • Independently ran the aggregate/classifier and partition modules: 78/78 passed, zero failures or skips, with cached Node 22.23.3 in an isolated offline container.
  • Repeated the original real-Git reproduction using the actual configuration contract, adding one valid entry from 165 to 166 while changing only config/dw-contract.php. Both the PR and verified merge-group cases now return run=true, with that file explicitly matched. No Rust code was executed.
  • Reviewed the 13 added input patterns and the inventory guards for literal native inputs, parity imports and workflow helper invocations. The classifier, aggregate and wrappers are byte-identical to the earlier reviewed revision, preserving the identity, current-main and full-qualification fallback guards. The workflow delta adds input patterns without changing fixture jobs, matrices, commands, permissions or resource limits.

No further finding in this reviewed delta. Hosted qualification still applies; an actual unchanged-input merge-group observation and measured timing remain outstanding under #830.

@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 28, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security review of corrected exact head e3026efd13225b52e69363ed99f290a3a6ce139c found no issue in the merge-group identity, command-injection, remote-detail, or fail-closed input-selection scope.

  • Pull-request head identity is compared rather than executed. Merge-group base/head values must be lowercase 40-hex identities, the target is fixed to refs/heads/main, HEAD must equal the event head, and the base must be an ancestor. Git receives an argument array without a shell, so object/ref input cannot become an option or command.
  • The classified diff is the complete event base-to-head range. A queued predecessor therefore remains visible; when a no-input candidate uses a synthetic predecessor as its base, the independent public-origin read cannot match current main and forces the full matrix. Missing/malformed identity, shallow history, non-ancestor history, moved/unavailable main, unsupported patterns and non-UTF-8 paths also force full qualification.
  • The only remote read is credentialless/read-only ls-remote, bounded to 30 seconds. Its stderr is suppressed and replaced with a fixed diagnostic, so a configured remote URL or transport detail is not echoed.
  • The repaired input declaration retains the prior patterns and now covers the audited root Cargo/checkout controls, config/dw-contract.php, external native fixture groups, all CI/conformance helpers, shared parity tests/support modules and the workflow itself. Literal native/import/workflow-helper inventory guards reduce future drift.

I independently ran the exact-head aggregate/classifier/partition suite from an isolated detached worktree: 78/78 passed (local Node 18.19.1; the owner separately recorded the required pinned/current tooling result). The worktree was removed afterward. Hosted current-head checks are green through the individual Shared matrix jobs; normal aggregate/queue qualification and #830's real unchanged-input merge-group observation remain the delivery evidence. This review adds no gate and requests no operator action.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Exact head e3026efd13225b52e69363ed99f290a3a6ce139c completed normal PR qualification: 56 successful checks and one conditional skip, with all 43/43 Shared Server fixture jobs passing in 40634531839. PHP, CodeQL, boundary and Sonar qualification passed as well.

The input-coverage finding is resolved in Cloud's independent exact-head review. Security's independent exact-head review found no issue within its stated identity/injection/remote-detail/input-selection scope. Both independently passed all 78 cases.

Normal merge-queue qualification is now running for combined head 30ee67fd5feae4407998dd4cbbe6e000bc304e24, based on accepted main 58655b70ce2c6ce9275029bdffbc8aa496ce93ff: Shared, PHP, boundary. This PR changes shared inputs, so the full matrix is expected. #830 remains open through actual unchanged-input merge-group selection/timing acceptance after merge; this result is not a fast-path measurement.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 28, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

The required merge-group attempt at 30ee67fd5feae4407998dd4cbbe6e000bc304e24 is terminal: 41 Shared fixture jobs passed, the MariaDB restart job failed, and the aggregate correctly failed. The queue entry and auto-merge request have cleared; this PR remains open.

The Rust owner diagnosed the failure on the published PHP reference: the replacement worker's own workflow completion preceded a typed no-task activity-poll capacity refusal and its subsequent withdrawal. The existing legacy-marker rule allowed that shutdown only for query pollers. #834 makes the fixture policy consistent for the three poll kinds while retaining the exact refusal envelope, accepted own commit, worker identity, withdrawal ordering and outcome/history requirements. General's exact-head review independently passes 2,585 contract controls.

#834 still needs normal hosted qualification, including resolution of its PHP feature failure in 40639879002. General is investigating that PHP gate; no failure-case attribution is claimed from the generic exit annotation. After the correction is qualified and merged, integrate the accepted source and qualify this PR through the normal queue. The original failure remains evidence; no timing-only retry or bypass has been used. #830's actual unchanged-input merge-group selection/timing acceptance remains outstanding.

@rmcdaniel

rmcdaniel commented Oct 28, 2026 •

Copy link
Copy Markdown
Member Author

Integrated current main, including merged #833 and #834, at 0ba815f02327ca1c46a7f520c7e783975132a5a1. The PR still changes only the three reviewed classifier/workflow/test files. Their blobs are identical to reviewed e3026efd1, including the workflow that passed pinned actionlint.

The combined source passes all 78 local Node contract cases: 72 input/aggregate cases and six part/deadline cases, zero failures or skips. Normal hosted checks are running at the new head. Sonar API review reports zero open issues and an OK quality gate; normal auto-merge is enabled for this exact head and will enter the required queue only after its checks pass. The original merge-group failure remains recorded; #834 supplies the accepted fixture correction rather than a timing-only retry.

The separate PHP feature failure from #834 is a confirmed product defect under #789. General's #835 repairs the committed-run/pre-summary payload deletion window; its focused controls pass on all four engines and related coverage passes 165 tests / 2,290 assertions. That repair remains independently qualified and does not change this selector's inputs or required checks.

Next: finish normal head and merge-group checks here, then observe a real unchanged-input PHP merge group such as #835. Job selection and timing acceptance for #830 remain pending. No speedup is claimed yet.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel
rmcdaniel enabled auto-merge October 28, 2026 07:06
@rmcdaniel
rmcdaniel added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 1bc5852 Oct 28, 2026
57 checks passed
@rmcdaniel
rmcdaniel deleted the fix/325-merge-group-fixture-inputs branch October 28, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant