test: make drift output test independent of colour detection

[rrama]

Pull Request Submission Checklist

• Follows CONTRIBUTING guidelines
• Commit messages are release-note ready
• Includes detailed description of changes
• Contains risk assessment (Low)
• Highlights breaking API changes (if applicable) — none
• Links to automated tests covering new functionality — the change is the test
• Includes manual testing instructions
• Updates relevant GitBook documentation — n/a
• Includes product update to be announced in the next stable release notes — n/a, test-only

What does this PR do?

Makes test/jest/unit/lib/iac/drift/drift.spec.ts → Test describe output › test output for known analysis pass regardless of the terminal it runs in.

That test compares getHumanReadableAnalysis output against the fixtures/all.console golden file, which contains hard-coded ANSI escapes (\e[1m…\e[22m, \e[34m…\e[39m). Whether those escapes are emitted at all is decided by chalk from the ambient environment, so today the test passes only when chalk happens to think colour is supported.

Concretely: chalk 2.4.2 depends on supports-color 5.5.0, which returns level 0 for a non-TTY stream before it gets as far as checking CI vendor variables. CircleCI runs steps under a pty, so the test passes in CI. Anywhere jest output is piped — a plain local run, an editor-integrated runner, a container, a sandboxed agent — chalk emits plain text and the test fails with a large and fairly baffling whitespace-looking diff:

-   Managed Resources: [1m8[22m
+   Managed Resources: 8

This wasn't hypothetical: it's reproducible on a normal dev machine with full network access and valid credentials, and it was originally reported as an environment problem because the failure looks nothing like a colour issue.

The fix pins chalk's colour level for that describe block and restores the previous value afterwards, so the assertion covers the formatting logic rather than the terminal it happens to run in.

Where should the reviewer start?

The whole diff is ~16 lines in drift.spec.ts. Worth confirming you agree with the direction: this keeps the golden file asserting on colours (rather than stripping ANSI from both sides, which would silently stop testing them).

How should this be manually tested?

npx jest --runInBand test/jest/unit/lib/iac/drift/drift.spec.ts                 # passes (fails on main)
FORCE_COLOR=1 npx jest --runInBand test/jest/unit/lib/iac/drift/drift.spec.ts   # passes
FORCE_COLOR=0 npx jest --runInBand test/jest/unit/lib/iac/drift/drift.spec.ts   # passes (fails on main)

On main the first and third fail. Full npm run test:unit was also run: this suite passes and nothing else changed behaviour.

What's the product update that needs to be communicated to CLI users?

None — test-only change, no shipped code touched.

Risk assessment: Low

Test-only. The chalk level is scoped to one describe block and restored in afterAll, and jest gives each test file its own module registry, so no other suite is affected.

Made with Cursor

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[PeterSchafer]

Question: has this been an issue you experienced somehwere? Otherwise such a global state change will become a problem when we start parallelizing things. Maybe NO_COLOR=1 for the CLI subprocess makes more sense!

[cursor]

Stale comment

Verification Report

Ran the 4-agent verification pass (semantic analysis, adversarial review, security scan, independent code review) against this PR's diff (test/jest/unit/lib/iac/drift/drift.spec.ts, +17/-0).

Verdict: PASS — no findings from any agent.

• Semantic analysis: No findings. Verified against the actual chalk@2.4.2 source that pinning chalk.level = 1 produces exactly the ANSI byte sequences in fixtures/all.console, that beforeAll/afterAll correctly capture and restore the ambient state, and that Jest's per-file module isolation rules out cross-file leakage.
• Adversarial review (Skeptic/Architect/Minimalist): No findings. Confirmed there's no terminal-detection logic in getHumanReadableAnalysis being masked by this fix, and that forcing a deterministic chalk level is the correct fix for the described flakiness rather than a workaround.
• Security scan: No findings — test-only change, no security-relevant surface (no manifest changes, no untrusted input, no new sinks).
• Independent code review: No findings. Minimal, correctly scoped fix; restoration runs unconditionally in afterAll regardless of test outcome.

Auto-approval policy

This PR's change is a test-only fix (drift.spec.ts), not a dependency bump (e.g. go.mod/package.json/lockfiles). Per this automation's policy, only dependency-only changes are auto-approved — so no automatic approval was applied here, despite the clean verification pass. This is a neutral, comment-only review; it does not block or change the mergeability of the PR.

  

_{Sent by Cursor Automation: Automatic PR verification}

[snyk-pr-review-bot]

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Global State Mutation 🟡 [minor] The test modifies the global chalk singleton properties level and enabled. While the PR attempts to restore them in afterAll, this can still lead to race conditions or side effects if other tests in the same process rely on chalk's default environment detection. A safer approach in Jest is often to mock the module or use a local instance if the library supports it, though for chalk v2/v4, this global mutation is a common workaround. const originalLevel = chalk.level; const originalEnabled = chalk.enabled; beforeAll(() => { chalk.level = 1; chalk.enabled = true; }); afterAll(() => { chalk.level = originalLevel; chalk.enabled = originalEnabled; });

📚 Repository Context Analyzed This review considered 4 relevant code sections from 4 files (average relevance: 0.78) src/cli/commands/describe.ts (lines 1-80) CLAUDE.md (lines 94-229) CONTRIBUTING.md (lines 135-311) dangerfile.js (lines 1-111)

🤖 Repository instructions applied (from AGENTS.md)

[cursor]

Verification Report

Re-ran the 4-agent verification pass (semantic analysis, adversarial review, security scan, independent code review) against this PR's current head (470ba5a), diff unchanged in substance from the previous round: test/jest/unit/lib/iac/drift/drift.spec.ts (+17/-0).

Verdict: PASS — no blocking findings from any agent.

• Semantic analysis: No findings. Verified capture/restore ordering is correct (values captured in the describe body before beforeAll runs), confirmed afterAll restores state even if a hook throws, and confirmed via chalk 2.4.2 source that chalk.level doesn't affect the two styles used here (bold/blue have fixed codes) — so the fixture can't mismatch on level choice. Also empirically proved no cross-file leakage: ran this spec alongside another file that mutates chalk.enabled without restoring it, with --runInBand (worst case for leakage) — both suites passed independently, confirming Jest's per-file module isolation.
• Adversarial review (Skeptic/Architect/Minimalist): PASS. The Skeptic empirically injected two regressions into the production code under test and confirmed both were caught (the fix doesn't weaken detection). The Architect directly investigated the global-mutation concern raised in the review thread below by @PeterSchafer — built a two-file leak probe and confirmed no contamination is possible across spec files; scoping to a single describe block with afterAll restoration is correct. Only low-severity, non-blocking suggestions remain (e.g. the codebase's existing toContainText/ANSI-stripping matcher solves a related-but-different problem and isn't a drop-in replacement here, since this test needs to verify exact ANSI placement).
• Security scan: No findings — test-only change, zero manifest diffs, no secrets, no new attack surface.
• Independent code review: No findings. Confirmed both required chalk gates (level and enabled) are pinned (chalk's applyStyle requires both), and the fixture's raw bytes are basic ANSI (bold/blue), matching chalk.level = 1 exactly.
• Local reproduction: independently ran npx jest test/jest/unit/lib/iac/drift/drift.spec.ts with default (piped) output, FORCE_COLOR=0, and FORCE_COLOR=1 — all 19 tests pass in every mode, confirming the fix resolves the flakiness as described.

Auto-approval policy

This PR's change is a test-only fix (drift.spec.ts), not a dependency bump (e.g. go.mod/package.json/lockfiles — none touched, confirmed via diff and the PR's own security/snyk check reporting no manifest changes). Per this automation's policy, only dependency-only changes are auto-approved, so no automatic approval was applied here despite the clean verification pass. This is a neutral, comment-only review; it does not block or change the mergeability of the PR.

  

_{Sent by Cursor Automation: Automatic PR verification}