Skip to content

test(installer): make ACP shim race deterministic - #11523

Closed
cjagwani wants to merge 1 commit into
codex/10947-acp-review-repairsfrom
codex/pr11458-ci-race-message
Closed

test(installer): make ACP shim race deterministic#11523
cjagwani wants to merge 1 commit into
codex/10947-acp-review-repairsfrom
codex/pr11458-ci-race-message

Conversation

@cjagwani

Copy link
Copy Markdown
Collaborator

Outcome

The ACP shim replacement-race test now deterministically exercises the intended identity-change path instead of occasionally observing a reused inode and accepting a different safe failure path.

Reason

PR #11458's Linux CLI shard failed because the test removed the original shim before creating its replacement. The filesystem could reuse the deleted inode, causing the installer to report that the shim was no longer managed instead of reporting an identity change. Both outcomes fail closed, but the fixture asserted only the identity-change diagnostic.

Related issues

Part of #11458.

Changes

  • Create the replacement symlink before the installer reaches the injected race.
  • Atomically move that prebuilt symlink over the managed shim so the replacement has a provably distinct identity.
  • Preserve the existing assertions that the installer fails, leaves the replacement target untouched, removes temporary files, and emits the intended identity-change diagnostic.

Verification

  • npx vitest run --project integration test/install/install-npm-resolution.test.ts --coverage=false — 29/29 tests passed.
  • npx oxfmt --check test/install/install-npm-resolution.test.ts — passed.
  • npx oxlint test/install/install-npm-resolution.test.ts — passed.
  • Normal pre-commit hooks — passed.
  • node --max-old-space-size=8192 node_modules/typescript/bin/tsc -p tsconfig.cli.json --incremental — passed after building the repository's CLI and plugin artifacts.
  • GitHub commit verification — commit is Verified.
  • Diff inspection — no secrets, API keys, or credentials.

Review notes

This PR intentionally targets codex/10947-acp-review-repairs, the source branch of #11458, and contains only the test-fixture correction for its failing CLI shard. No production or user-facing behavior changes, so no documentation update is needed.


Signed-off-by: Charan Jagwani cjagwani@nvidia.com

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cjagwani cjagwani self-assigned this Sep 11, 2026
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0514e671-b1b0-4bdc-a943-883baacb1a75

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

@github-code-quality

github-code-quality Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 600375b in the codex/pr11458-ci-rac... branch remains at 96%, unchanged from commit 72c66fb in the codex/10947-acp-revi... branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 600375b in the codex/pr11458-ci-rac... branch remains at 83%, unchanged from commit 189043e in the codex/10947-acp-revi... branch.

Show a line coverage summary of the most impacted files.
File codex/10947-acp-revi... 189043e codex/pr11458-ci-rac... 600375b +/-
src/lib/actions...oy-execution.ts 94% 91% -3%
src/lib/gateway...ntime-action.ts 89% 86% -3%
src/lib/actions...dbox/destroy.ts 91% 89% -2%
src/lib/adapter...shell/client.ts 91% 90% -1%
src/lib/actions...-add-restart.ts 31% 30% -1%
src/lib/onboard...eway-service.ts 82% 82% 0%
src/lib/onboard...uild-context.ts 75% 75% 0%
src/lib/onboard...eway-process.ts 89% 90% +1%
src/lib/adapter...-acp-ssh-cli.ts 86% 88% +2%
src/lib/adapter...nfig-capture.ts 0% 100% +100%

Updated September 11, 2026 06:18 UTC

@cjagwani

Copy link
Copy Markdown
Collaborator Author

Superseded by a7786bc on #11458, which directly addresses the same Linux inode-reuse test failure. I re-reviewed that exact delta and reran the focused installer file: 29/29 tests pass. Closing this alternate fixture-only correction to avoid duplicate paths.

@cjagwani cjagwani closed this Sep 11, 2026
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