Skip to content

Reuse the current native debug binary across parity profiles - #875

Merged
rmcdaniel merged 4 commits into
mainfrom
fix/871-native-debug-artifact
Oct 30, 2026
Merged

rmcdaniel merged 4 commits into
mainfrom
fix/871-native-debug-artifact

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Thirty corpus/profile jobs rebuild the same debug Server executable. This change builds it once per workflow run and shares an executable-only artifact with those non-native-test cells. Every existing matrix case, native Cargo test command and bootstrap/schema/catalog observation remains.

The producer uses the pinned compiler with locked/offline inputs, clears restored first-party outputs, and uploads only server, build.json and SHA256SUMS with one-day retention. Consumers require a successful producer and exact run/producer-attempt artifact name, then verify checkout SHA, native tree, lock/compiler, repository/run/attempt, platform/profile and hashes before installing. Missing or mismatched artifacts fail. There is no fallback build or cross-run executable cache. Native-test cells retain source builds and the successful-main dependency-cache writer.

Independent native tests still execute if the debug producer fails and SDK adapters succeed; the aggregate fails for a failed producer. Failed-consumer retries may reuse the successful producer attempt from the same run, while full producer reruns have distinct artifact names. Workflow-owned cleanup removes only the two explicitly scoped containers.

Validation at 598ed24:130 focused Node tests/no skips, Bash syntax, actionlint/whitespace, Docker isolation scan and74 policy tests pass. Controls execute identity/tamper/refusal, PATH-hijack, producer preflight, cleanup and exact native Cargo/sharding argv; non-native parts run without Cargo and reject a missing binary. Hosted Shared40978654861 passes 44/44, including all30 consumers and all8 native-test cells. The complete PR has57 successful checks and1 conditional skip; native-owner and Security reviews accept this exact head. Normal protected queue entry:00:50:56 UTC, behind #870. Merge-group/main verification remains pending.

Measured hosted comparison: the30 consumer jobs total21,643s versus24,351s on the same-native-tree baseline; the new producer adds138s. The overall workflow is32m59s versus26m30s and has larger job-start delays, so no overall speedup is claimed. Five baseline SQLite builds total463s; the candidate builds once in108s for all30 parts. Runner/load variance prevents sole-cause attribution.

Refs #871. Earlier failed intermediate attempts and their corrections remain in the PR evidence. Required checks, native source/runtime and frozen fixtures are unchanged.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security helper-only review at exact c7c397191efb11d65869e4e7115c2761929ad7c4: no correction requested for this checkpoint.

The producer and verifier bind the binary to the exact checkout SHA/native tree, Cargo lock, repository, workflow run and producer attempt, pinned compiler image, runner architecture, debug profile and bounded build resources. Creation refuses a dirty native tree or pre-existing output, cleans first-party package output, builds --locked --offline as the runner UID with no network, and exports only server, build.json and SHA256SUMS. Consumption rejects another run/source/compiler/architecture, a future producer attempt, extra payload files, symlinks, changed identity metadata and binary/checksum tampering. The strengthened tests recompute metadata checksums for identity-tamper cases and cover pre-container refusal plus failed-build cleanup. The PR's reported 22 focused tests are appropriate for this helper checkpoint; hosted checks are still running.

This is not final workflow acceptance. Review the integration separately for pinned upload/download action SHAs, read-only permissions, same-run artifact selection, GitHub digest failure plus the local identity/checksum verifier before execution, one-day retention, exact producer dependency/attempt plumbing and cleanup. Native tests/native-tests-b, their Cargo commands, the successful-main cache writer, evidence capture and aggregate result must remain unchanged. No cross-run compiler/workspace payload or speedup claim is accepted.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head review: accepted at 5e911ad with no Security hold.

The same-run artifact remains limited to the exact checkout/compiler/run/producer attempt and three-file payload; consumers fail closed on missing, failed, future, or wrong-run producer identity before pinned download, then re-verify metadata and checksums before install. Native test parts retain their independent Cargo path when the producer fails.

The earlier 2b11d99 hosted failures were real integration findings, not waivers: dynamic CID cleanup was replaced with an exact run/attempt/job RESOURCE_SCOPE and workflow-owned always cleanup, and the existing part contracts now explicitly require the five non-native preparation parts while preserving one-part assertions for actual checks. Independent correction checks passed: Docker workflow isolation policy and 108/108 focused Node tests, with no skips; diff check is clean.

Hosted current-head qualification is still running and the PR is behind main after #874, so normal base update and hosted results remain required. This review adds no separate gate.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head re-review of 598ed24c5861e724a99f59b9af845934181cb968 over current base 9670e0378d37ef751c59757c8d35284639c71a2f: no Security finding or hold.

The sole new-code Security-rating finding is corrected at source: both artifact-identity Git lookups use fixed /usr/bin/git paths, and the executed negative control places a failing fake git first in PATH while verification succeeds without invoking it. The adjacent digit-class, explicit-dispatch, and Bash-guard changes are behavior-preserving Sonar cleanup; they do not broaden the artifact boundary or alter failure behavior.

Independent exact-head checks:

  • git diff --check 9670e037...598ed24c: pass
  • node --test tests/Unit/NativeDebugArtifactTest.mjs: 30/30 pass, including same-run/attempt identity, checksum and symlink refusal, producer-failure isolation, scoped ephemeral containers, three-file payload, pinned download/digest verification, and retained native-test builds

The previously accepted artifact controls remain intact. The older 5e911ad hosted producer success is evidence only for that older head; current-head Sonar and normal hosted qualification remain pending and should be read on their own merits. This accepts the exact source correction without adding a separate release or Rust-development gate.

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust Server owner review of exact head 598ed24c against my four conditions:

  1. Exact build identity (met). native-debug-artifact.mjs checks:

    • GITHUB_SHA == HEAD, a clean committed rust/ tree, the HEAD:rust tree id and the Cargo.lock digest;
    • the pinned NATIVE_RUST_IMAGE, run id and attempt, OS and arch;
    • --locked --offline, and the binary's sha256.

    Each consumer recomputes that identity from its own checkout and verifies build.json, SHA256SUMS and the binary before installing it. The parts run the binary in the same pinned image.

  2. Fail closed (met). A consumer refuses before downloading on any of: a non-success producer, a malformed attempt or one in the future, or a mismatched artifact name. download-artifact uses digest-mismatch: error, and the verifier refuses extra files and non-regular files. There is no cached or main fallback.

  3. Native tests unchanged (met). native-tests and native-tests-b keep their cargo commands and still run when the producer fails (!cancelled() && published-sdk-adapters success). The SQLite codec and cross-decode step stays in native-tests.

  4. Same evidence (met). Bootstrap, schema-check and catalog comparison still run /target/debug/durable-workflow-server in each part, and each consumer saves the verified identity as parity-evidence/native-debug-build.json.

One check: the producer builds only --bin durable-workflow-server, where the parts ran a full cargo build before. No non-test part uses another built binary or example: the only /target/debug path is that binary, and --example codec-fixtures runs only in native-tests. So that's fine.

No finding from me. The hosted qualification decides the merge.

@rmcdaniel

rmcdaniel commented Oct 30, 2026 •

Copy link
Copy Markdown
Member Author

Current combined head is 598ed24c5861e724a99f59b9af845934181cb968, integrating main #874 (9670e037) and the scoped Sonar correction. Both identity-related Git lookups now use /usr/bin/git; a failing fake Git placed first in PATH is never executed by the verification control. New-code digit-class, dispatch and Bash-guard findings were also corrected within these new helpers.

Local combined qualification passes 130 Node tests, no failures/skips, plus Bash syntax and the merged-source Docker isolation scan. The preceding policy correction passed all 74 Docker-policy tests; that policy code is unchanged. Actionlint/whitespace pass. Every native Cargo test command, sharding selection, matrix case and bootstrap/schema/catalog stage remains. Non-native prep is explicitly limited to the five existing corpus/profile parts, and independent native test execution survives a producer failure. No policy waiver or check removal.

Earlier hosted failures are retained: 2b11d993 failed Public Boundary 40975267094 because dynamic/helper cleanup was not an accepted workflow scope, and Repository contracts found the missing non-native-preparation classification and obsolete unconditional-build assertion. Corrected 5e911ad subsequently passes hosted boundary/contracts. Its Sonar gate exposed the PATH-based Git lookup, now fixed at the current head with an executed negative control.

There is an initial real producer positive on 5e911ad only: job 123808198690 succeeds. Build66s, full job97s, verification and thin-artifact upload succeed. This is not final-head or complete consumer qualification. Fresh hosted checks for598ed24 are automatic. Before merge/completion, verify the final producer, all required consumer/native cells, source/identity retention and actual timing observations. No overall speedup is claimed from this partial receipt.

Final-head qualification is complete: producer123813335675 succeeds on598ed24 (00:18:19–00:20:37 UTC, build108s,total138s). The full Shared run40978654861 passes44/44, including all30 actual artifact consumers and all8 native-test cells. Complete PR checks:57success/1conditional skip; exact-head native-owner/Security reviews accept. Protected queue entry00:50:56 UTC, position2 behind #870. Merge-group/main follow-through is pending.

Complete same-native-tree timing comparison and limits: affected consumer+producer elapsed is2,570s lower in these observations, while overall workflow time is389s longer with larger start delays. No overall speedup, pure-queue metric or billing-minute claim. Earlier intermediate failures above remain retained in their original scopes.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Current-head follow-up for 598ed24c5861e724a99f59b9af845934181cb968: the SonarQube job and external SonarCloud check are now SUCCESS. Fresh quality-gate readback is OK: new-code security, reliability, and maintainability ratings are all A (1), duplication is 0.0%, and security-hotspot review is 100%. Repository contracts and both public-boundary scans are also green.

This verifies that the fixed-executable correction cleared the exact Security-rating failure identified on 5e911ad. Security has no remaining hold; the rest of the hosted suite and CI wall-time evidence remain the normal owning qualification.

@rmcdaniel

rmcdaniel commented Oct 30, 2026 •

Copy link
Copy Markdown
Member Author

Security merge-group source and producer review for 05a68180c2dee8fb9604888ab9154a5cbe81f212:

  • Its parents are merged main 6c1e63dd2f9aae8ed93620a57fdba80851c43dab (Fix update-validation probe timing across retries #870) and this PR's accepted exact head 598ed24c5861e724a99f59b9af845934181cb968.
  • Comparing the merge-group commit to the accepted PR head adds only Fix update-validation probe timing across retries #870's already-qualified timer accounting change, its focused test, and the corresponding comparator test-list entry/reformat.
  • native-debug-artifact.mjs, native-debug-build.sh, the artifact tests, aggregate/parts controls, and the PR's same-run identity/transport implementation are unchanged from the accepted head. The combined workflow introduces no new credential, permission, retention, executable-source or cross-run cache boundary.
  • Merge-group producer job 123832740718 succeeded. Every validation/build/thin-artifact/upload/cleanup step passed.
  • The merge-group run exposed exactly one current artifact: native-debug-build-40984639344-1, API digest sha256:de95b52d73285399346966e2c4d8032653c369f3895912a047fafb13ec88b9b5, 23,704,780 bytes, bound to run 40984639344 and head 05a68180..., expiring after 24 hours on October 31. No compiler cache is present in that handoff.

Final protected and trusted-main follow-through:

  • Merge-group Shared Server fixtures run 40984639344 and PHPUnit run 40984639343 both completed successfully with no failed job. GitHub then merged this exact queue commit, unchanged, at 2026-10-30 01:48:49 UTC.
  • Trusted-main run 40988042415 is a push on branch main at the exact merged SHA. Its producer job 123843606305 passed pinned-compiler, validation, attempt-binding, thin-artifact, upload and cleanup steps.
  • That main run exposes exactly one current artifact, native-debug-build-40988042415-1, API digest sha256:57cc018e4035ab55fec86228ed38df82f45c9708aac8c63ab354b905b0d907ba, 23,704,781 bytes, bound by API metadata to run 40988042415, branch main and merged SHA 05a68180..., with the intended 24-hour expiry.
  • All 30 expected non-native artifact consumers completed both the same-run fetch and verify/install steps successfully. The eight native-tests / native-tests-b cells intentionally skip artifact download under the workflow's explicit condition and its source test; they remain the separate native qualification cells, not missing consumers.

Security's source, protected merge-group and trusted-main artifact-identity acceptance is complete. Trusted-main run 40988042415 then completed successfully with all 44 jobs terminal and zero failures at 2026-10-30 02:12 UTC. This acceptance makes no overall speedup claim and changes no product, release, provider or operator authority.

Merged via the queue into main with commit 05a6818 Oct 30, 2026
58 checks passed
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