Skip to content

test: cover codex-multi-auth wrapper#307

Open
ndycode wants to merge 2 commits intomainfrom
test/pr14-codex-multi-auth-wrapper-smoke
Open

test: cover codex-multi-auth wrapper#307
ndycode wants to merge 2 commits intomainfrom
test/pr14-codex-multi-auth-wrapper-smoke

Conversation

@ndycode
Copy link
Owner

@ndycode ndycode commented Mar 22, 2026

Summary

  • add a focused smoke test for scripts/codex-multi-auth.js
  • verify the wrapper loads the built CLI entry and forwards args with the package version env
  • improve coverage for a previously untested CLI entrypoint script

Validation

  • npm run typecheck
  • npm run lint -- test/codex-multi-auth-wrapper.test.ts
  • npm run test -- test/codex-multi-auth-wrapper.test.ts
  • npm run build
  • manual QA: node scripts/codex-multi-auth.js auth --help

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

adds a focused smoke test for scripts/codex-multi-auth.js that verifies the wrapper correctly injects the package version into CODEX_MULTI_AUTH_CLI_VERSION and forwards cli args to runCodexMultiAuthCli. the previous concern about bare fs.rm in cleanup is fully resolved — the test now imports and uses removeWithRetry from the shared helper.

  • removeWithRetry is correctly used in afterEach with recursive: true, force: true — windows antivirus/ebusy safety is satisfied
  • pop()-based cleanup loop preserves unprocessed entries in tempRoots if an earlier cleanup throws, which is actually more resilient than the splice(0) pattern in the sibling test
  • the readFileSync block (lines 60-65) re-reads the copied script and asserts it contains "runCodexMultiAuthCli" — this is tautological and only tests that copyFileSync worked; the behavioural assertions on stdout are sufficient
  • spawnSync result is used without first checking result.error, making spawn-level failures produce cryptic assertion messages
  • exit-code normalization (non-integer → 1, non-zero integer propagation) is not tested here, but those paths are already covered by codex-multi-auth-bin-wrapper.test.ts

Confidence Score: 4/5

  • safe to merge — the one prior blocking concern (bare fs.rm) is resolved; remaining notes are non-blocking style improvements
  • the main issue flagged in the previous review round is fully addressed with removeWithRetry. the test correctly exercises the version-env and args-forwarding paths. two p2 style issues remain (tautological readFileSync assertion, missing result.error guard) but neither affects correctness or windows safety.
  • no files require special attention

Important Files Changed

Filename Overview
test/codex-multi-auth-wrapper.test.ts new smoke test for the codex-multi-auth wrapper; correctly uses removeWithRetry (addressing the prior thread concern), but includes a tautological readFileSync assertion and lacks a result.error guard before status assertions.

Sequence Diagram

sequenceDiagram
    participant T as test (vitest)
    participant FS as tmp dir (mkdtempSync)
    participant W as codex-multi-auth.js (copy)
    participant M as dist/lib/codex-manager.js (stub)

    T->>FS: mkdtempSync → root
    T->>FS: write package.json { version: "9.9.9-test" }
    T->>FS: copyFileSync scripts/codex-multi-auth.js → root/scripts/
    T->>FS: writeFileSync stub codex-manager.js → root/dist/lib/
    T->>W: spawnSync(node, [wrapper, "auth", "--help"], { cwd: root })
    W->>FS: createRequire → require("../package.json") → "9.9.9-test"
    W->>W: process.env.CODEX_MULTI_AUTH_CLI_VERSION = "9.9.9-test"
    W->>M: import runCodexMultiAuthCli(["auth","--help"])
    M-->>W: logs version + args, returns 0
    W-->>T: stdout, status=0
    T->>T: assert stdout contains "version=9.9.9-test"
    T->>T: assert stdout contains "args=auth --help"
    T->>FS: afterEach removeWithRetry(root)
Loading

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: test/codex-multi-auth-wrapper.test.ts
Line: 60-65

Comment:
**tautological `readFileSync` assertion**

reading the copied script back and asserting it contains `"runCodexMultiAuthCli"` only verifies that `copyFileSync` reproduced the source file — not any runtime behaviour of the wrapper. the string is hardcoded in `scripts/codex-multi-auth.js` and will always be true. consider replacing or dropping it; the `stdout` assertions on lines 57-58 already give you meaningful behavioural coverage.

```suggestion
		// The stdout assertions above cover the meaningful behaviour;
		// no need to re-read the script file here.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/codex-multi-auth-wrapper.test.ts
Line: 55-59

Comment:
**unguarded `spawnSync` result**

if the node process fails to spawn (bad path, missing exec, etc.), `spawnSync` sets `result.error` and leaves `result.status` as `null`. asserting `toBe(0)` on a null will produce a cryptic failure. a quick guard at the top of the assertion block makes failures much easier to diagnose:

```suggestion
		expect(result.error, "spawnSync failed").toBeUndefined();
		expect(result.status).toBe(0);
		expect(result.stdout).toContain("version=9.9.9-test");
		expect(result.stdout).toContain("args=auth --help");
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: "Use retry-safe clean..."

Context used:

  • Context used - AGENTS.md (source)
  • Context used - test/AGENTS.md (source)

@chatgpt-codex-connector
Copy link

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 22, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • skip-review

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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a939621a-d09e-441d-a237-2a2186cc0e8c

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/pr14-codex-multi-auth-wrapper-smoke
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch test/pr14-codex-multi-auth-wrapper-smoke

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 and usage tips.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant