feat: gate ordinary target links on typed compile admission outcomes - #178
Conversation
Add an explicit standard target API over the existing typed batch and shared compile wave. Preserve original per-source failures, callback exceptions, unentered work, split inspection/miss execution and post-execution freshness retry. Failure outranks concurrent stop; interruption before terminal-link admission prevents link/link-cache publication. Successful individual TU cache saves and already-admitted terminal links keep their original behavior. Legacy no-token path and CLI callers stay unopted. Add 84 product-boundary checks with real cache files and deterministic process tools plus four policy-batch checks. No process kill token, project write-lease claim, VERSION or release change.
Resumed exact-source verification — product cache tests and real-tool regressions are distinctReconfirmed base Actual Windows logs were inspected in Debug job 102730455603 and Release job 102730020368: the target test compiled/linked/ran 84 new checks and its enclosing executable passed; the scheduler executable ran 50 batch checks and passed. The target checks use the actual product target/compile/link/cache coordinators and real cache files, with a deterministic ProcessRunner. They are NOT true cl.exe runs of the new target entry point. The native C4100 unused Eight core artifact ZIPs were downloaded and SHA256/CRC checked. Independent replay confirms default 70/70, private 42/42, all required positive controls, ownership-record 150/150 expectations, and the unchanged six true-MSVC two-worker batch regressions. The real batch runs preserve two original C2338/exit2 failures as failed despite concurrent cancellation, successful siblings and no successor link/run/sentinel; they validate the underlying adapter, not a new-target end-to-end compiler path. Exact input executable bytes match their own source/provenance. The default matrix retains nine adverse managed-ending B failures and private four. All original diagnoses/exits/unattempted link/run states and separate recoveries remain; all28A-started managed endings still kill the original service. These negative-policy cases do not waive a failed positive control; current required positive controls actually pass. Audited705/409original tool results and1410/818diagnostic files; B owner identity15/70 and5/42 remains incomplete, not a quiescence certificate. Original observer-OFF ABBA #593/34431700856 actually built both exact refs and measured all76pairs. All72instrumented complete counter/breakdown/hit-miss identities match. External timings-off no-op paired median +0.0983ms/+1.0498363% does not cross the original BOTH>1ms AND>10% gate. Preserve cold/single-TU/link-only 4/4 slower medians +239.579ms/+18.85%, +51.755ms/+56.07%, +17.678ms/+31.82%; scale-cold maximum +2847.057ms. No attribution, zero-overhead claim, deletion as noise or historical debt closure. Four external counter sets are unavailable, not zero; four-pair P95 is merely a maximum. Current code preserves the post-execution shared-dependency barrier and conservative freshness retry. Failed/cancelled opt-in waves never enter target link/cache-save, but a successful TU may retain its own valid cache; this is not project rollback. Late stop after terminal link admission does not terminate or undo that link. CLI/static/module/PCH-parent cancellation remains outside this change. Final merge decision follows full scope and protection review; VERSION5.5.0 and #172 remain unchanged. |
Iviesever
left a comment
Iviesever
left a comment
There was a problem hiding this comment.
Final review — accept explicit ordinary-target compile admission; keep runtime and transaction limits
Implementation self-review with separate read-only source reconstruction, native-log inspection and artifact/statistics replay. This is not an independent human approval. Resumed the already-published PR rather than recreating its branch or replacing its code.
Exact base: 57fb087a036e513552555257a6d37ae7126e1108.
Reviewed head: dbb76bc88b173804dcc2711e499f806e2b8c9190.
Candidate tree: 81e15a7507f4b972df8535322a943d4ecf9b731a.
Native test-merge: 3ccd1c0ec3715ed06a1ade98724ed3b0d4df0dce, identical tree and exactly those base/head parents.
Complete six-file diff reviewed: 497 additions/56 deletions,414files in each tree,408byte-identical. Both canonical source trees were reconstructed and the patch reproduces the candidate. Public headers and existing target/wave implementations change, so this is product integration, not a test-only diff. Scheduler implementation, compiler/link/cache implementations, CLI/static/module callers, source manifest,79-executable native inventory, benchmark/ABBA, observer tools and VERSION remain unchanged.
Verified blobs: batch header 49a2e2b1f6f6ffda93a8cbb7098b61282ddfbce9; target header d534449d94ff713c6c1d34c20c614734c6e7625d; target implementation 56c42ea88ecd5ab732dfdd4efa23ed9e09828253; wave 7b668c886597611002df45aa4bbf3f49430c2f8a; target tests 55a591b23a0a8e6eb170eab7bcfb445087f53f87; batch tests 88f85a205676b022ec004605c5382fdb9d1d9727.
Product control-flow review
run_with_compile_admission_stop validates before checking admission cancellation; a non-stoppable token delegates the ordinary run. The ordinary implementation is a false compile-time specialization, retaining original scheduler calls without allocating typed phase/evidence vectors. The refactor is not claimed zero-cost: it changes compiled product code and the public error layout.
Small/forced builds retain direct compile execution. Larger targets keep shared-evidence inspection, only execute misses, suspend inspection TLS during execution and revalidate shared dependencies after miss execution. The existing conservative whole-target retry remains; ordinary compile failure does not trigger retry. A stopped second wave retains both the earlier completed wave and current attempts, including source-index maps for noncontiguous misses.
Actual returned compile errors are selected before concurrent cancellation or sibling scheduling exceptions; original exception_ptrs and successful sibling results remain in the attached report. Cache hits, inspected misses and unentered sources are not conflated. Failure/cancellation returns before the existing link coordinator is invoked, so its target link-cache save is not reached. Successful individual TUs can still retain valid per-source caches. This is not atomic project rollback or proof that external writers have stopped.
The final stop read is the documented terminal-link admission decision. After it passes, a later stop does not terminate link or undo its valid cache save. No ProcessSpec receives a token. Static/modules/PCH-parent orchestration and CLI remain unadopted. The endpoint is not noexcept or an exhaustive OOM-recovery API: allocations outside the caught wave/evidence attachment may still throw; partially launched native-worker startup failure has not been injected.
Actual Windows execution, not just status
All nine applicable first-attempt workflows completed successfully:
- Native Debug #733/34431700798: actual build, all four paired subshards, ambient/include/runtime/parameter and freshness/side-output checks, final gate. In job102730455603, subshards2/3, the target executable actually compiled/linked/ran 84 new target checks and passed; batch checks actually ran 50 and passed.
- Native Release #620/34431700817: Stage0, all four Release shard jobs, full self-host, runtime-only staging and validation of the exact packaged artifact. The same84target/50batch checks actually passed in job102730020368. publish-release skipped.
- Typed MSVC Batch #2/34431700821, Default Endpoint #24/34431700813, Private Ownership #26/34431700826 passed their original real-tool gates.
- Documentation #181/34431700804, Cross-Stage #210/34431700845, Build Plan #81/34431700795 passed.
- Independent Performance #593/34431700856 actually checked out both exact refs, built both Release binaries, verified instrumentation, measured all ABBA pairs and uploaded them.
Accepted File Event/PDB Open studies were not triggered by unchanged workflow paths; they are not labeled current rerun passes. No post-publication source correction or same-head native/CI/benchmark retry was requested. The native C4100 unused inspection parameter warning in the false wave specialization is retained as a nonblocking warning; this is not a warning-free/WX build claim and it was not silently patched while reusing these results.
What the new84checks prove, and what they do not
The new target tests execute the actual target/compile/link/cache coordinators and real filesystem cache persistence, using a deterministic ProcessRunner instead of tools. They verify real target link-cache bytes AND mtime unchanged under cancellation, compile failure, callback exception, combined failure+exception and process infrastructure error. Positive cold builds actually enter the product linker and save the cache; warm hits, missing objects and missing final output preserve existing rebuild behavior. A failed source cannot publish a new successful source-cache entry; a successful sibling may save its own valid entry.
Tests also cover pre-stop without writes, invalid request priority, sparse miss/source mapping, inspection failure before execution, missing tasks distinct from hits, TLS suspension, post-execution header mutation requiring conservative rebuild, interruption during that rebuild retaining both waves, small/forced path and late-stop-after-terminal-admission behavior. None of this substitutes a fixture sentinel for the actual product link-cache file.
This is not yet a true-cl.exe run of the new target API. The unchanged six-case real-MSVC batch fixture independently exercises the underlying batch adapter, not this new target endpoint. Its Debug/Release complete/cancel/failure-cancel outcomes remain succeeded/cancelled/failed, preserving original compiler exits0,0,0 /0,0 /0,2(C2338) and the injected marker. Both positive cases link/run/commit a fixture sentinel; all four stopped cases retain item2 not_started and no successor.18original results/36diagnostics and12source/12planned-argv comparisons were checked. Exact batch input binary bytes match provenance. Recipe argv is not OS command telemetry, and the injected static_assert is not a historical PDB failure reproduction.
Original ownership regressions and adverse outcomes retained
Independent raw audit confirms default70/private42 complete, all required unmanaged/direct/single-worker scheduler positive controls and 150/150 record expectations (113accepted/37rejected). Exact collector CRLF hash matches the native record. All14scheduler/direct input pairs match56source files and70argv records after only directory normalization; actual prebuilt inputs and seven-helper/refusal outputs are checked.
Default preserves nine adverse managed-ending B failures, private four, with original C1090/C1051/C2471/C1001 diagnostics and unattempted link/run when appropriate. Every later recovery remains separate. Across both modes all28A-started managed endings still kill the original shared service. These are intentionally retained unsafe-policy tests, not a failed required positive control. Audited 705default/409private tool results and1410/818stdout-stderr records. Resource identity appears15/70default and5/42private; missing observations do not prove writer quiescence.896/524binary inventory entries are post-case hashes, not retained generated binary bodies or failure-time snapshots.
Independent unchanged ABBA — all19scenarios
Artifact 10134877895,35346bytes,SHA256 bfdecf9adf3ef823f6bfbfbf4639b35a77b93e0e241e3d227a4c96d4a64c3496. Recomputed all76pairs, stored pair deltas, AB/BA/AB/BA order, paired medians/MAD/nearest-rankP95 and marginal medians. 72/72 instrumented complete counters/breakdowns/compile-link-archive hit-miss identities match; four external timings-off counter sets are unavailable, not zero. Only the last row uses external elapsed; all others use mqb.timings. Four-pairP95 is a maximum, not a population-tail estimate.
| Scenario | Paired median Δms | Paired median Δ% | MAD ms | Paired P95 Δms | Slower |
|---|---|---|---|---|---|
| cold | +239.579 | +18.85 | 133.132 | +853.079 | 4/4 |
| no-op | +0.132 | +4.43 | 0.158 | +1.035 | 2/4 |
| single-tu | +51.755 | +56.07 | 22.454 | +84.419 | 4/4 |
| public-header | +45.071 | +51.21 | 26.604 | +83.224 | 3/4 |
| build-run | +0.808 | +6.92 | 1.239 | +2.920 | 3/4 |
| link-only | +17.678 | +31.82 | 16.369 | +177.081 | 4/4 |
| target-scale-cold | +0.386 | +0.02 | 247.459 | +2847.057 | 2/4 |
| target-scale-no-op | +0.867 | +3.97 | 0.610 | +4.203 | 3/4 |
| target-scale-no-op-auto | +2.932 | +14.71 | 3.768 | +7.599 | 3/4 |
| target-scale-no-op-j1 | +2.799 | +11.18 | 5.121 | +29.487 | 2/4 |
| target-scale-common-header-no-op | +0.488 | +2.09 | 0.552 | +5.278 | 2/4 |
| target-scale-single-tu | +10.325 | +11.04 | 21.916 | +136.806 | 3/4 |
| discovery-cold | −47.799 | −3.57 | 55.540 | +41.415 | 1/4 |
| discovery-no-op | −0.017 | −0.34 | 0.009 | −0.006 | 0/4 |
| discovery-header | −22.765 | −18.85 | 17.489 | −4.392 | 0/4 |
| modules-cold | −193.744 | −11.49 | 211.709 | +48.349 | 1/4 |
| modules-no-op | −0.054 | −1.33 | 0.010 | −0.040 | 0/4 |
| timings-enabled-no-op | −0.067 | −2.49 | 0.025 | −0.040 | 0/4 |
| timings-disabled-no-op (external) | +0.098 | +1.05 | 0.150 | +0.291 | 2/4 |
Unrounded external medians +0.0983ms/+1.0498363% do not cross the unchanged predeclared BOTH>1ms AND>10% gate. This does NOT make the instrumented regressions disappear: cold,single-TU andlink-only are slower4/4, and scale-cold has a+2.847second adverse tail. No cause is isolated; neither source causality nor harmlessness is established by these measurements. No speedup/zero-overhead claim, offset against favorable module/discovery values, presumed-noise deletion, AA subtraction or historical debt closure. The comparison JSON does not preserve the actual performance binary pair or OS trace.
Decision, artifacts and next boundary
Accept ordinary expected-head merge of dbb76bc88b173804dcc2711e499f806e2b8c9190, subject to unchanged main/protection. The previously declared current correctness and performance gates pass. This accepts a bounded, explicit product target API, not general CLI cancellation or a production-release reliability conclusion. Keep VERSION5.5.0, #172 independently held, M1b incomplete and historical C1041/#170/cold-tail debts open. No project transaction, shared-service termination, writer lease, build/run/session/residency or formal release authority is included.
Next separate small iteration should exercise this actual target entry point with real MSVC/link and actual target cache files, using fixed Debug/Release success/cancellation/failure-plus-cancellation cases and retained original diagnostics/cache identities. The current deterministic process tests and real underlying batch tests must not be relabeled that end-to-end coverage. Preserve inspection/execution/freshness gates and avoid adopting CLI/static/modules/PCH-parent cancellation before their boundaries are reviewed. Do not return to expanding observer tooling.
Eight current core ZIPs were SHA256/CRC verified and independently replayed. Default10135034570 SHA256 eb1fe01d2ad4da9789de882ff6238233049b2474ef7af2b912b878892ee3843b; private10134917038 617ea00c8c75c167b7873f54d59eec76c602bb5e4a69e9dbe8b72bf1f0ff79af; batch10134916054 5db512bc02050cfbf8f5dcaad21a3b8c2d99b08c5b4b59ac036636d41202d4e1. Inputs,150case contracts,build,complete source archives,patch and portable replay are retained. This is not every Actions artifact; native execution logs in delivery are labeled excerpts, not full CI log archives. Actions retention30days.
Publication-stage local GCC/shim tests and mutation checks are described in the original PR record; resumed acceptance independently verified current source and actual Windows logs rather than inventing another local48-TU run. Public-network access in the local container failed, so the exact source was reconstructed from connector-read content and validated by its Git tree. No local MSVC/PowerShell/ETW execution is claimed. No source/raw artifact was modified to make audits pass, and no same-head CI/benchmark retry occurred. Title/ready-triggered skipped performance jobs cannot replace actual#593; post-merge push CI is not claimed already completed here.
Iviesever
changed the title
1 participant
Final decision: accept the explicit ordinary-target API
Final exact-source review, native execution, complete ABBA table and scope-limited acceptance
Accept ordinary expected-head merge. This is product target/compile-wave integration, not a test-only diff, CLI Ctrl+C feature or write-lease authority. #172 remains independently held and historical C1041/performance debts remain open.
Exact source
57fb087a036e513552555257a6d37ae7126e1108.dbb76bc88b173804dcc2711e499f806e2b8c9190.81e15a7507f4b972df8535322a943d4ecf9b731a.3ccd1c0ec3715ed06a1ade98724ed3b0d4df0dce, exactly those base/head parents.Behavior and limits
MsvcIncrementalTargetCoordinator::run_with_compile_admission_stopis explicit opt-in; a non-stoppable token delegates the ordinary run. Validation keeps priority. A false compile-time specialization retains ordinary scheduling without typed evidence vectors; changed compiled code/public error layout are not claimed cost-free.The wave retains small/forced direct execution, large-target inspection/only-miss execution, TLS evidence ownership and the post-execution shared-dependency revalidation with conservative full rebuild. Original compile errors win over simultaneous cancellation/sibling exceptions. Evidence keeps source-indexed original results, unentered items, phase summaries, exception pointers and both waves if conservative retry stops. Existing numeric and new policy batch overloads share one result collector and the existing scheduler/resource policy.
A failed or cancelled compile wave returns before target link/cache-save. Successful TUs may still save their own valid caches; this is not project rollback. After the final terminal-stage admission check, later stop does not terminate link or undo its cache save. No token reaches ProcessSpec. Static/modules/PCH-parent orchestration and CLI cancellation remain separate. Setup/evidence allocations outside caught wave operations may throw; partial native-worker-start failure is not fault-injected.
Actual current-head validation
Accepted File Event/PDB Open investigations did not trigger and are not labeled new passes. No post-publication source revision or same-head CI/native/benchmark retry was requested. Native C4100 for unused
inspectionin the false wave specialization is retained as a nonblocking warning; builds are not described as warning-free.Product-boundary tests versus true compiler runs
The 84new tests execute the actual product target/compile/link/cache coordinators and real cache files, but tool processes are deterministic substitutes. They check target link-cache bytes and mtime unchanged on cancellation/original failure/exception, successful source-cache preservation without fake rollback, sparse miss maps, inspection errors, TLS suspension, execution-time header changes, conservative retry and terminal-link late-stop semantics. Positive paths really enter product link/cache persistence; this is not the old fixture sentinel renamed as integration.
The separate existing true-MSVC six-case fixture exercises the underlying typed batch, not this new target entry point. Both failure-cancel cases retain C2338/exit2 and successful siblings while reporting failed, never cancelled-success. Four stopped cases suppress pending work and fixture successors; both positive cases link/run.18original results/36diagnostics and exact input executable provenance were independently checked. True-cl end-to-end verification of the new target API remains the next focused step.
Original default/private matrices retain nine/four adverse managed-ending B failures, original diagnostics, unattempted link/run and separate recoveries. All28A-started managed endings still kill original services; sparse owner identity is not writer quiescence.705/409tool results and1410/818diagnostics audited.150ownership record expectations match113accepted/37rejected;14scheduler/direct input pairs match56sources/70argv after directory normalization only.
Performance and original gates
Resumed artifact/native verification
External timings-off no-op paired median +0.0983ms/+1.0498363%, max+0.291ms,2/4slower, does not cross the original BOTH>1ms AND>10% gate. All72instrumented counters/breakdowns/compile-link-archive hit-miss values match; four external counter sets remain unavailable, not zero. Only the external row uses timings-off elapsed; four-pairP95 is a maximum.
Preserve cold/single-TU/link-only 4/4slower with medians+239.579ms/+18.85%,+51.755ms/+56.07%,+17.678ms/+31.82%, plus scale-cold+2847.057ms tail. No cause has been isolated. No speedup/zero-overhead claim, offset against favorable scenarios, noise deletion, AA subtraction or historic debt closure. Comparison JSON does not archive the performance executable pair/OS trace.
Predeclared exact-source native/selfhost/package,84target+50batch tests, original70/42positive controls, six real underlying batch cases,150record contracts and original19x4ABBA/72semantic identities/unchanged threshold all pass. No required current failure was waived. Limited acceptance of the API is not a claim that historical C1041 or release performance requirements are solved.
Handoff and evidence retention
Next separate small iteration: exercise this target entry point with actual MSVC/link/cache files, keeping fixed Debug/Release success/cancellation/failure-plus-cancellation controls and original diagnostics. Do not relabel the present process-substitute target tests or the old real batch fixture as that coverage. Keep no-token behavior, freshness and inspection/execution boundaries, and do not expand CLI/static/modules/PCH-parent cancellation or observers prematurely. M1b, external-writer authority, transactions/build-run/session/residency and formal VERSION/release remain gated.
Eight immutable core artifact ZIPs, exact base/head source archives, patch, manifests and executed portable read-only replay are retained; not all Actions artifacts. Retention30days. Actual input executables match their own provenance, not the independent ABBA pair. Generated compiler binary bodies are omitted by the diagnostic contract; finite hash inventories are post-case, not failure-time snapshots. Native logs in delivery are marked excerpts. Resumed local verification reconstructed exact source through the connector after container network failure; it does not claim another local48-TU or MSVC/PowerShell/ETW run. Temporaryperf title removed only after actual#593completed; title/ready skips and post-merge push CI do not replace that evidence.