test(apply): cover refreshStaleEnvDefaults regression suite

[tonychang04]

Summary

Adds 13 unit tests for the env-merge logic that 0.1.68 introduced. Acts as regression guard against the fix — if anyone changes the matching/replace logic, tests will catch behavioral drift.

Coverage

extractEnvPairs:
  ✓ returns KEY → value pairs
  ✓ preserves URL-style values verbatim
refreshStaleEnvDefaults:
  ✓ replaces user value when it matches manifest default and platform has real value
  ✓ preserves user value when it differs from the manifest default
  ✓ skips refresh when platform has no real value (self-hosted, helper returned null)
  ✓ handles multiple keys, refreshing only the stale ones

All 13 pass locally (npx vitest run src/auth-providers/apply.test.ts → Tests 13 passed (13)).

🤖 Generated with Claude Code

---

Summary by cubic

Add 13 unit tests for the env-merge path introduced in 0.1.68, covering extractEnvPairs and refreshStaleEnvDefaults. Tests ensure we only replace values that still equal the manifest default when the platform has a real value, keep user customizations, and handle multiple keys.

^{Written for commit fd0f80a. Summary will update on new commits.}

Summary by CodeRabbit

• Tests
  • Expanded test coverage for environment variable handling and platform value synchronization functionality.

[coderabbitai]

Walkthrough

This PR expands test coverage for the auth-providers apply module by adding comprehensive unit tests for two helper functions: extractEnvPairs verifies parsing of KEY=value strings into a Map with URL value preservation, and refreshStaleEnvDefaults validates conditional logic for replacing stale environment defaults with platform-provided values across multiple scenarios.

Changes

Auth Helpers Test Coverage

Layer / File(s)	Summary
Test Utility Imports src/auth-providers/apply.test.ts	Import list expanded to include extractEnvPairs and refreshStaleEnvDefaults from ./apply.js.
extractEnvPairs Unit Tests src/auth-providers/apply.test.ts	New test block validates parsing of KEY=value strings into a Map structure and preserves URL-style values verbatim.
refreshStaleEnvDefaults Unit Tests src/auth-providers/apply.test.ts	New test block covers conditional refresh scenarios: replacing values matching manifest defaults with platform-provided real values, preserving divergent user values, skipping refresh when platform provides no real value, and refreshing only selected stale keys across multiple entries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• InsForge/CLI#106: Adds tests for the exact helpers (extractEnvPairs and refreshStaleEnvDefaults) and behavior implemented in this PR, so the changes are directly related.

Suggested reviewers

• jwfing

Poem

🐰 Hops through test cases with glee,
Env pairs parsed, stale defaults set free,
Platform values refreshed with care,
URL-style strings preserved fair,
Coverage blooms where functions play! 🌱

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding regression test coverage for the refreshStaleEnvDefaults function introduced in version 0.1.68.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/refresh-stale-env-defaults

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/auth-providers/apply.test.ts (1)

119-124: ⚡ Quick win

Strengthen replacement assertions to prove stale values are removed.

Current checks mainly use toContain, so an implementation that appends refreshed lines but leaves old placeholders could still pass for some keys. Add explicit negative assertions for stale values in the multi-key case.

Proposed assertion hardening

   const { updated, refreshed } = refreshStaleEnvDefaults(existing, defaults, platform);
   expect(refreshed.sort()).toEqual(['DATABASE_URL', 'INSFORGE_JWT_SECRET']);
   expect(updated).toContain('DATABASE_URL=postgresql://cloud@host/db?sslmode=require');
+  expect(updated).not.toContain('DATABASE_URL=postgresql://postgres:postgres@127.0.0.1:5432/insforge');
   expect(updated).toContain('BETTER_AUTH_SECRET=user-set-this-already');
   expect(updated).toContain('INSFORGE_JWT_SECRET=real-jwt-secret-from-platform');
+  expect(updated).not.toContain('INSFORGE_JWT_SECRET=replace-with-output-of-cli-secrets-get-JWT_SECRET');

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/auth-providers/apply.test.ts` around lines 119 - 124, The test for
refreshStaleEnvDefaults should assert that stale placeholder lines are removed,
not just that new lines were added; update the test in apply.test.ts to add
negative assertions for the original stale values present in the existing
fixture (e.g., the old DATABASE_URL placeholder and the old INSFORGE_JWT_SECRET
placeholder) by adding expect(updated).not.toContain(...) checks alongside the
current positive checks so refreshStaleEnvDefaults is proven to replace, not
merely append, those entries.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/auth-providers/apply.test.ts`:
- Around line 119-124: The test for refreshStaleEnvDefaults should assert that
stale placeholder lines are removed, not just that new lines were added; update
the test in apply.test.ts to add negative assertions for the original stale
values present in the existing fixture (e.g., the old DATABASE_URL placeholder
and the old INSFORGE_JWT_SECRET placeholder) by adding
expect(updated).not.toContain(...) checks alongside the current positive checks
so refreshStaleEnvDefaults is proven to replace, not merely append, those
entries.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a0453a95-4f9a-497b-94bf-be315ecc3e36

📥 Commits

Reviewing files that changed from the base of the PR and between e94452a and fd0f80a.

📒 Files selected for processing (1)

• src/auth-providers/apply.test.ts

[cubic-dev-ai]

No issues found across 1 file