Skip to content

fix(windows): stabilize credential inspection and test subprocesses - #662

Merged
mldangelo-oai merged 1 commit into
mainfrom
mdangelo/codex/fix-windows-credential-acl-churn
Aug 26, 2026
Merged

fix(windows): stabilize credential inspection and test subprocesses#662
mldangelo-oai merged 1 commit into
mainfrom
mdangelo/codex/fix-windows-credential-acl-churn

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix the two Windows failures in the latest main CI run: interrupted credential ACL enumeration during concurrent ambient imports, and a Python test subprocess that returned no exit status.

Changes

  • Recognize the specific Get-ChildItem file-disappearance error and retry the complete credential ACL snapshot through the existing three-attempt loop. Finish validating streamed descriptors before starting another attempt.
  • Keep unrelated subprocess failures, unsafe permissions, missing credential homes, and persistent churn fail-closed. Retain the eight concurrent ambient imports in the existing test.
  • Add deterministic regressions for recovery after interrupted enumeration, exhaustion of the existing retry limit, and immediate propagation of unrelated ACL subprocess failures.
  • Await the Windows scan-root alias test's Python process with execFile, retaining its timeout and output assertions. This small fixture change also appears in the broader CI work in ci: balance test shards and stabilize CI checks #651.

Testing

  • The full hosted CI run passed on dbfd8dca6a: 32 checks passed and 5 release-only checks skipped. This includes all Linux, macOS, and Windows runtime lanes, package checks, both previously failing Windows shards, and the machine-wide PowerShell policy check.
  • Failing-first regression: two of the three new cases failed before the runtime fix; all three pass afterward.
  • Focused runtime and credential checks with seeds 638943413 and 12345: each passed 27 tests with 5 Windows-only skips and 0 failures.
  • pnpm run types: passed.
  • pnpm run format: passed.
  • pnpm run build: passed.
  • git diff --check: passed.
  • The pinned native dependency was recovered after an initial download failure and verified against the lockfile integrity hash; no dependency or lockfile changes were made.
  • Full local pnpm run test --seed 12345 under restricted process access: 1,891 passed, 30 skipped, and 6 failed. Five long-running fixture/subprocess timeouts passed focused reruns; the sixth was an explicitly sandbox-denied ps call.
  • Full local pnpm run test with normal process access, seed 3628695636: 1,897 passed, 29 skipped, and 1 failed. The existing CLI-export asynchronous-write test exceeded its three-second deadline; an immediate isolated replay and five further isolated repetitions passed. That unrelated test is unchanged. These local full-suite attempts are not claimed green.
  • Exact-head Codex review completed without findings.

Risk and rollout

The production change is limited to the Windows credential ACL inspection path. Only the observed file-disappearance error requests a fresh snapshot; permission and other inspection failures still reject the operation. No public CLI, dependency, timeout, retry-limit, or CI-matrix changes. Unix behavior is unchanged. This PR does not merge or release the fix.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@github-actions github-actions Bot added the bug Something isn't working label Aug 26, 2026
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review the exact head dbfd8dc, including the narrowly classified Windows ACL retry, fail-closed behavior, and unchanged concurrent-import coverage.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-26T11:08:05.660684Z dbfd8dc Draft marked ready
🔒 Security Review Completed 2026-08-26T11:08:57.904076Z dbfd8dc Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: dbfd8dca6a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review August 26, 2026 11:06
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

Final validation for dbfd8dca6a4f894caf4c7d8114bd61f6eb025dea:

  • All test shards passed on both full CI runs, including both Windows failures that prompted this PR.
  • The follow-up workflow's first attempt hit the existing 180-second timeout during the Windows/Node 24 package consumer's npm install, before the installed-package tests ran. Only the failed job and its dependent checks were retried on the unchanged commit; attempt 2 passed.
  • No test coverage, timeouts, or retry limits were changed to obtain the passing result.
  • Exact-head Codex code and security reviews completed without findings. The local full-suite failures and successful isolated reruns remain documented in the PR's Testing section; those local full-suite attempts are not claimed green.

The PR is ready for review and has not been merged.

@mldangelo-oai
mldangelo-oai requested review from faizan-oai and kmbroai and removed request for kmbroai August 26, 2026 15:07
@mldangelo-oai
mldangelo-oai merged commit f5e98f8 into main Aug 26, 2026
90 of 93 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-windows-credential-acl-churn branch August 26, 2026 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants