Repository navigation
Reuse the current native debug binary across parity profiles - #875
Conversation
|
Security helper-only review at exact 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 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. |
|
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. |
|
Security exact-head re-review of The sole new-code Security-rating finding is corrected at source: both artifact-identity Git lookups use fixed Independent exact-head checks:
The previously accepted artifact controls remain intact. The older |
rmcdaniel
left a comment
There was a problem hiding this comment.
Rust Server owner review of exact head 598ed24c against my four conditions:
-
Exact build identity (met).
native-debug-artifact.mjschecks:GITHUB_SHA == HEAD, a clean committedrust/tree, theHEAD:rusttree id and theCargo.lockdigest;- 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,SHA256SUMSand the binary before installing it. The parts run the binary in the same pinned image. -
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-artifactusesdigest-mismatch: error, and the verifier refuses extra files and non-regular files. There is no cached or main fallback. -
Native tests unchanged (met).
native-testsandnative-tests-bkeep their cargo commands and still run when the producer fails (!cancelled() && published-sdk-adapters success). The SQLite codec and cross-decode step stays innative-tests. -
Same evidence (met). Bootstrap, schema-check and catalog comparison still run
/target/debug/durable-workflow-serverin each part, and each consumer saves the verified identity asparity-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.
|
Current combined head is 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: 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. |
|
|
Current-head follow-up for This verifies that the fixed-executable correction cleared the exact Security-rating failure identified on |
|
Security merge-group source and producer review for
Final protected and trusted-main follow-through:
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. |



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.jsonandSHA256SUMSwith 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.