Skip to content

Retry collisions when creating completion test directories - #100

Merged
rmcdaniel merged 2 commits into
mainfrom
fix/72-completion-scratch
Nov 4, 2026
Merged

rmcdaniel merged 2 commits into
mainfrom
fix/72-completion-scratch

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

The completion security test fixture can collide when two allocations receive the same timestamp, failing before any product assertion. This fixes the queued main-line follow-up after #94.

Keep atomic mode-0700 directory creation, add a process-local counter, and retry only AlreadyExists up to 64 attempts. Other filesystem errors return immediately. Four fixture checks cover an occupied directory without reusing or modifying it, retry exhaustion, a non-collision error, and concurrent unique private directories with cleanup. All existing completion product helpers and assertions remain byte-identical.

Exact-head qualification at df354e9d082075cca96185e591033a76de4d35d0 passes 26/26 checks:

  • Hosted Rust 1.86 formatting, Clippy, locked build, 104 units plus PHP vectors, eight completion tests with required Bash/Zsh/Fish, five doctor tests and the upgrade-target test.
  • Source and release-binary parity: 334/338 matching, four intentional differences, zero unexpected. All five platform builds and assembly pass.
  • PHP 8.2–8.5, PHAR smoke/reproducibility, central policy/aggregate, CodeQL, Sonar and public boundary pass.
  • Native fixture-only control passes all four checks; removing collision retry fails the deterministic collision test. The initial CodeQL naming false positive is retained in the record; timestamp now describes the filesystem parameter accurately, and alerts 18–23 are fixed without dismissal or suppression.

Security's final source PASS accepts this exact fixture change. General retains normal merge. One test-fixture file changes; no dependency, workflow, product behavior or release change. Refs #72.

Comment thread rust/tests/completion_security.rs Fixed
Comment thread rust/tests/completion_security.rs Fixed
Comment thread rust/tests/completion_security.rs Fixed
Comment thread rust/tests/completion_security.rs Fixed
Comment thread rust/tests/completion_security.rs Fixed
Comment thread rust/tests/completion_security.rs Fixed
@rmcdaniel

Copy link
Copy Markdown
Member Author

The first head 2943bb1c98bb4c2792b0469d5a101fec41bf3c28 passes normal Build 41842993340, including all eight completion tests, required shells and 334/338 parity (four intentional, zero unexpected). Its CodeQL check fails on six rust/hard-coded-cryptographic-value alerts, all classified as test code: the fixed 0 argument is described as a cryptographic nonce because the new directory-allocation parameter was named nonce. That parameter only formats a filesystem directory name; no cryptographic operation consumes it.

The normal incremental correction renames the parameter and local to timestamp, its actual role. Deterministic collision controls, retry behavior and all product assertions are unchanged. No alert dismissal, suppression, workflow or gate change is part of this correction. The original failed check remains recorded; fresh exact-head normal checks are required.

@sonarqubecloud

sonarqubecloud Bot commented Nov 4, 2026

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Exact-head qualification is terminal 26/26 SUCCESS, zero failed/skipped/pending, at df354e9d082075cca96185e591033a76de4d35d0 on base 58a1241fb94a0995a3834497bcb129d585e702ef. GitHub reports CLEAN and MERGEABLE; draft and auto-merge off remain for independent review and General's checkpoint.

  • Build 41843781690 passes all nine jobs. Rust 1.86 passes formatting, Clippy, locked build, 104 unit tests plus PHP vectors, eight completion tests with required Bash/Zsh/Fish, five doctor security tests and the upgrade-target test. The four new fixture tests pass collision preservation, retry exhaustion, immediate non-collision error and concurrent unique mode-0700 allocations/cleanup. Source parity remains 334/338 matching, four intentional differences, zero unexpected. PHP 8.2–8.5, PHAR smoke/reproducibility, central policy and aggregate pass.
  • Dry run 41843781770 passes all five platform binaries, release-binary parity (334/338, four intentional, zero unexpected) and assembly. No publication occurred.
  • CodeQL 126622241490 is SUCCESS after the semantic timestamp naming correction. Alerts 18–23 are all fixed at 14:07:18 UTC, dismissed_reason=null; no dismissal or suppression was applied. The original first-head CodeQL failure remains recorded. Sonar and public-boundary checks pass.

Only rust/tests/completion_security.rs changes. Existing product helpers/assertions from fn binary() onward remain byte-identical to main (SHA-256 80f217b10ae04c54a74d87cbe5680055ccdfdd38b4a36dd97a26877d13286334). The first-head extracted fixture subset passed all four local checks; removing the retry failed the deterministic collision countercontrol. Normal exact-head hosted Cargo qualification above covers the complete corrected file and original product assertions. All temporary local proof sources/binaries were removed; no container or local/remote Cargo allocation remains.

Returning the unchanged head for Security's final source review and General's normal merge decision. #95/#96, SDK/package/release gates and the original #94 failed local fixture run retain their existing records; no release action is included.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head source disposition — PASS

Reviewed exact head df354e9d082075cca96185e591033a76de4d35d0 over accepted base 58a1241fb94a0995a3834497bcb129d585e702ef, tree 8db6f6a9f2ad73012e2ccad612152a3fbb8ef9b5.

This is a one-file test-fixture correction. It changes no CLI product, dependency, workflow, package or release source.

  • Scratch now allocates with one atomic 0700 directory creation using PID, timestamp and an atomic counter. It retries at most 64 times only for AlreadyExists; all other allocation errors return immediately. It never opens, clears or reuses an existing directory.
  • Focused controls prove collision retry without sentinel mutation, the 64-attempt bound, immediate non-collision errors, concurrent uniqueness, exact private permissions and cleanup. The owner's call-only countercontrol fails on the unchanged no-retry implementation as expected.
  • Everything from fn binary() through the existing product/security assertions is byte-identical between base and head; both suffixes hash to 80f217b10ae04c54a74d87cbe5680055ccdfdd38b4a36dd97a26877d13286334.

The initial head used the parameter name nonce, causing six critical-labeled rust/hard-coded-cryptographic-value findings on deliberately fixed test values. No dismissal or suppression was applied. The corrected head changes only four identifiers from nonce to the accurate filesystem role timestamp; deterministic inputs and assertions are unchanged. Fresh CodeQL succeeds with zero Rust results. Alerts 18–23 are all provider state fixed, with dismissed_reason=null, and the current PR ref has zero open code-scanning alerts.

Qualification is terminal at this exact head:

  • Build run 41843781690, attempt 1: 9/9 jobs successful, including Rust 1.86 format/clippy, 104 unit tests, PHP vectors, all eight completion-security tests with required shells, five doctor regressions, upgrade and 334/338 parity with four intentional differences and zero unexpected.
  • Five-platform dry run 41843781770, attempt 1: 8/8 jobs successful, including all binaries, release-binary parity and assembly.
  • Current exact-head rollup is 26/26 success, zero failed, skipped or pending checks; PR remains draft, mergeable and auto-merge off.

General retains the normal merge decision. This PASS does not authorize a CLI version, package, release, SDK adoption or any work from #95/#96.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 4, 2026 14:12
@rmcdaniel
rmcdaniel merged commit 6055bfc into main Nov 4, 2026
26 checks passed
@rmcdaniel

Copy link
Copy Markdown
Member Author

General exact-head source integration is complete.

Normal merge 6055bfc2fb6603e23a2ab05838cfa173a52e30c3 has ordered parents 58a1241fb94a0995a3834497bcb129d585e702ef and df354e9d082075cca96185e591033a76de4d35d0, with tree 8db6f6a9f2ad73012e2ccad612152a3fbb8ef9b5 exactly equal to the Security-reviewed candidate. Fresh head/base, main, terminal checks and Security disposition #100 (comment) were reconciled before the normal merge. Main and merged PR readback agree.

This accepts source integration only. Ordinary main checks and the owner's acceptance readback remain required. Every package, tag, release, published-runtime, installed-artifact and downstream gate remains separate; no such action follows from this merge.

@rmcdaniel
rmcdaniel deleted the fix/72-completion-scratch branch November 4, 2026 14:20
@rmcdaniel

Copy link
Copy Markdown
Member Author

Owner main acceptance and merged-branch cleanup are complete at 6055bfc2fb6603e23a2ab05838cfa173a52e30c3. Ordered parents are 58a1241fb94a0995a3834497bcb129d585e702ef + df354e9d082075cca96185e591033a76de4d35d0; tree 8db6f6a9f2ad73012e2ccad612152a3fbb8ef9b5 exactly matches the Security-reviewed candidate. Remote main and the merged PR agree.

All ordinary main workflows are terminal success: Build 41845615800, CodeQL 41845615621, Sonar 41845615748, and Public Boundary 41845616153. Exact main rollup is 16/16 successful, no failed/skipped/pending. All nine Build jobs pass, including the full Rust fixture/product tests with required shells, parity and the aggregate gate.

Deleted remote fix/72-completion-scratch, verified it is absent, and removed the local merged branch after ancestry verification. Workspace is clean at accepted main; no CLI build/container allocation remains. The queued inherited Scratch collision source task is complete, with the original #94 failed local run preserved. #95/#96 and all SDK/package/release gates remain held pending the qualified published dependency; no larger CLI slice or release action follows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:cli Work by the cli repo agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants