Skip to content

Stabilize custody and native provider readiness fixtures - #72

Merged
MrScripty merged 4 commits into
mainfrom
fix/catalog-custody-admission-20261014
Oct 14, 2026
Merged

MrScripty merged 4 commits into
mainfrom
fix/catalog-custody-admission-20261014

Conversation

@MrScripty

@MrScripty MrScripty commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

This PR repairs three test-fixture readiness races without changing production custody or process semantics. The catalog fixture now separates checkpoint cleanup from target-reader admission. The native managed-Python fixtures distinguish early provider completion from slow descendant startup before exercising cancellation/timeout and exact process drain.

Head ee1948ef6e44e7b4b854d1155d74d347e00708f7, based on main 694c6f676d367342dc33e8f46140e635d2df9f63:

  • 29bc30b4d1c150e4cd0751f73c84dd5e3f13969e: five test-only catalog baseline lines.
  • b43bf6881e7a04f7f1b8d86f5e566bcc22799973: managed-Python readiness helper, two focused regressions and native fixture callers/script; all inside the existing test module.
  • 299c95afcaa3f486208f999182404541b4c7b9de: regenerated default/S3 attribution inventories. Only this managed-source input hash changed; all 378/428 package records and both notice texts are byte-identical.
  • ee1948ef6e44e7b4b854d1155d74d347e00708f7: Linux/x86_64 positive post-shutdown reopen helper and two focused regressions; exactly three admitted callers use it.

Scoped plan: inspect exact failing jobs and owner ordering, reproduce where this executor can run, change only affected test fixtures, preserve original custody/exit/drain assertions, run focused and affected native cohorts plus formatting/byte-scope checks, obtain independent review and exact-head native Windows CI. No merge or release is authorized by this continuation.

Catalog evidence:

  • Main job 116085425753 failed at catalog.rs:378, then holder unwind produced catalog_snapshot.rs:151 RecvError.
  • Unchanged local main reproduced both exact panics on repetition 230 (229 passed, 1 failed). Repaired test passed 1,000/1,000 repetitions.
  • Complete no-default library unit cohort at 29bc30b: 2,070 passed, zero failed, 13 ignored. PR Build 38638858630 subsequently passed its headless job and seven other applicable jobs; only Windows native QA failed at the separate fixture below.

Windows fixture finding and repair:

  • Failed job 116109192183 first passed the managed-Python-specific tests, then both descendant fixtures failed during the broader cargo test --locked --manifest-path rust/Cargo.toml -p pumas-app-manager native_ cohort: 31 passed, 2 failed, 10.76s. Both failures were at the old 10s marker wait, before cancellation/drain assertions.
  • Main's identical managed-Python source passed that 33-test Windows cohort in 8.75s (job 116085425733). The old helper did not inspect provider completion, so its log cannot retrospectively distinguish early failure from slow startup; neither is fabricated as a proven production defect.
  • Readiness now races the marker against the actual provider JoinHandle and reports its returned error/join outcome immediately. PowerShell fixture errors terminate promptly via $ErrorActionPreference = 'Stop'.
  • Startup budget is 20s, leaving the unchanged 5s exact-handle acquisition within the existing 30s cancellation-provider deadline. The timeout fixture retains its 60s deadline and advances the clock only after descendant custody. Existing process-handle/exit, cancellation, drain/cleanup, timeout and closed-admission assertions are unchanged.
  • New controlled tests prove an early provider error remains diagnostic and a marker arriving after 12 virtual seconds can be admitted without consuming/completing the provider task. Those are helper-contract tests, not native Windows process evidence.

Windows-readiness local validation at b43bf68/299c95af (before the Linux-only reopen helper):

  • cargo test --locked --offline --manifest-path rust/Cargo.toml -p pumas-app-manager version_manager::managed_python::tests: 16 passed, zero failed/ignored, 0.23s, on Linux.
  • cargo test --locked --offline --manifest-path rust/Cargo.toml -p pumas-app-manager native_: 52 passed, zero failed, 2 ignored, 4.14s, on Linux. No Windows qualification is inferred from this.
  • Local build uses Rust 1.92.0, a task-owned target, debug info disabled and incremental compilation off to bound disk use. Dependency resolution is unchanged.
  • cargo fmt --manifest-path rust/Cargo.toml --all -- --check, git diff --check, unchanged lockfiles and exact production/assertion byte comparisons passed.
  • Default/S3 inventories were regenerated using the canonical generator; both attribution checks passed, and the full Node release suite passed 37/37, zero failures/skips.
  • Managed-provider production prefix SHA-256: 16bb1973250fc9ba4997e4db7c273fb2ee6e0c7d62feafe215bcf517ff86534c. Catalog production prefix remains 8e07cdf8a50377a0dbade66bfe8d26fbfba11107d9a1bbf8b8b537b28c4f6c14; Cargo lock remains 91fb9823de2f0346211b4f68ca400f03a628890e55dca90e0803e2dd7ad47e42.

Build 38648293478 stopped at attribution freshness before native jobs: the complete managed-provider source file is a recorded attribution input. The derived inventory update fixes that exact stale hash without changing notices/dependencies. Exact-head Build 38648696810 is in progress. The PR remains draft; independent parent review of this concrete final diff and actual native Windows outcome remain pending. No self-review or Linux result is called independent/native Windows acceptance.

Known remaining source gate: Rust quality job 116142054658 reached the full app-manager library cohort and failed 326 passed / 1 failed / 3 ignored. Unchanged fixture version_manager::tests::native_adopted_output_survives_legacy_or_mismatched_cleanup_custody reopened at version_manager/mod.rs:2377 and received InvalidParams: Pumas library instance is already running for physical store. That file was not edited. A subsequent local full app-manager library run passed 327/327, 3 ignored, but does not clear this CI failure. That source gate is now addressed by ee1948e, with exact-head CI still required; this PR is not yet merge-ready. Windows provider-specific tests passed; the later broader native cohort was skipped when that superseded run was cancelled by the next source push.

Positive post-shutdown reopen repair:

  • Parent independently diagnosed and admitted the three positive Linux/x86_64 reopen sites formerly at mod.rs:2372/2605/2874. StoreLifetime documents that another test's fork can retain the native shared descriptor until exec; local shutdown/drop does not prove an immediate nonblocking reacquire. The builder refuses physical-store busy before startup mutations.
  • Only these positive callers now wait up to 10s in 5ms intervals, matching the existing core fixture policy. Retry requires both PumasError::InvalidParams and the complete physical-store-busy message for the exact canonical root. Every other result/error returns immediately; an expired busy error remains an error. No production unlock, retry, PID/claim deletion or custody relaxation.
  • A real API is shut down but its physical share is retained under a controlled gate: the helper stays pending until that share is dropped, then reopens. A second regression verifies another constructor error returns unchanged without advancing paused retry time and preserves the invalid root's bytes.
  • Exact byte reconstruction proves all original mod.rs content/assertions/refusal tests is unchanged apart from the three factory substitutions; the helper/regressions are inside Linux/x86_64 test cfg. Production mod.rs prefix SHA-256 remains 58caf0b7c1def59ff6b42101b162b17376f0a07e2c1321b8b77cec8cf2f43e06.
  • Final local checks before push: 2 focused regressions passed; cargo test --locked --offline --manifest-path rust/Cargo.toml -p pumas-app-manager native_ 54 passed / 2 ignored; complete -p pumas-app-manager --lib 329 passed / 3 ignored on three consecutive runs, zero failures (11.34s / 11.11s / 10.97s). App-manager cargo clippy --locked --offline --manifest-path rust/Cargo.toml -p pumas-app-manager --all-targets -- -D warnings, formatting, both attribution checks and scope/lock checks passed. Logs: /tmp/pumas-positive-reopen-{focused,native,clippy}-final.log and /tmp/pumas-positive-reopen-full-final-{1,2,3}.log.

Review and exact-source disposition: independent parent source review accepted 299c95a. The new ee1948e helper needs its own parent review. Final-head Build 38652452559 has been requested automatically by the push and is pending. The earlier Windows readiness job on 299c95a, 116142054566, was automatically cancelled by the existing per-PR concurrency rule after ee1948e was pushed. Before cancellation, its managed-Python-specific cohort passed 14/14, including both new readiness regressions and both real descendant cancellation/timeout drain fixtures (0.48s). The later broader native_ cohort was skipped, so it is not qualified. Final-head Windows job 116154691514 is running; final-head workflow/attribution checks passed, and Rust quality is rerunning. The ee1948e diff is Linux-only fixture code, without a claim of identical binaries or automatic transfer of CI acceptance. Independent parent review of ee1948e and terminal final-head CI remain pending. PR stays draft and unmerged.

Canonical standards main was refreshed at 188beda1fa477d21d576c233dd8c7f4c4c267d23; no applicable AGENTS.md/.agents/skills exists. Task-local evidence logs are /tmp/pumas-catalog-{baseline-repeat,fixed-repeat,core-no-default}.log and /tmp/pumas-managed-readiness-{local,native-local}.log, not release assets.

Preserve release/v080-package-qualification-20261014 at exact 9dadd33f1b906032c6102516aff03a08ae35c78c. No API, runtime timeout, dependency pin, user inference work, other repository, tag/release or denied access changes. Full package/trained-model release gates remain open.

Worktree /workspace/Pumas-Library-catalog-custody-20261014 is retained/protected by fix/catalog-custody-admission-20261014 at this head for review and native CI; parent integration owner decides disposition afterward.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 22dbe992-e26c-40f4-bfa0-7cd7fb3f6554
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MrScripty MrScripty changed the title Fix catalog custody test admission race after checkpoint Stabilize catalog custody and native provider readiness fixtures Oct 14, 2026
@MrScripty MrScripty changed the title Stabilize catalog custody and native provider readiness fixtures Stabilize custody and native provider readiness fixtures Oct 14, 2026
@MrScripty
MrScripty marked this pull request as ready for review October 14, 2026 23:49
@MrScripty
MrScripty merged commit 9ea60b7 into main Oct 14, 2026
28 checks passed
@greptile-apps

greptile-apps Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The reviewed changes appear safe to merge, subject to the required final-head CI checks.

Summary

This PR changes test fixtures and recorded attribution hashes, not production behavior.

  • The catalog fixture waits for task cleanup before measuring queued readers.
  • Native provider fixtures wait for child startup and spot early exits.
  • Version-manager fixtures reopen after the old store share is released.

No actionable issues were found. Review checks covered source and diff whitespace; no tests or native processes were run during this review.

Reviews (1) · Last reviewed commit: "test(custody): await physical-store rele..." · Reviewed by Greptile

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