test: config — ConfigPaths.parseText substitution and JSONC parsing

[anandgupta42]

Summary

ConfigPaths.parseText — src/config/paths.ts (16 new tests)

This function is the foundation of all config loading in altimate-code. It handles {env:VAR} environment variable substitution, {file:path} file content injection, and JSONC parsing with error reporting. Zero tests existed prior to this PR — confirmed via grep -r "parseText\|{env:\|{file:" packages/opencode/test/ returning no results.

If this code breaks:

• API keys injected via {env:ANTHROPIC_API_KEY} silently become empty strings
• Credential files referenced via {file:~/secrets/key.txt} stop being read
• Config parsing errors lose line/column info, making debugging impossible

Specific scenarios covered

JSONC parsing (5 tests):

• Plain JSON object parsing
• JSONC comments are stripped correctly (jsonc-parser invoked, not JSON.parse)
• Trailing commas allowed (regression guard for allowTrailingComma: true option)
• ConfigJsonError thrown on invalid syntax with correct error shape (e.data.path)
• Error messages include line and column info for user debugging

{env:VAR} substitution (4 tests):

• Environment variable value is substituted correctly
• Missing env var falls back to empty string (the || "" path)
• Multiple env vars in same config text all get replaced (global regex)
• Raw text injection — env vars are NOT JSON-quoted, enabling {env:NUM} → number

{file:path} substitution (5 tests):

• Absolute file path reads and trims content
• Relative file path resolves from config file's directory (not process.cwd())
• ConfigInvalidError thrown for missing files with "does not exist" message
• missing='empty' parameter returns empty string instead of throwing
• {file:...} tokens inside // comments are skipped (comment detection logic)
• File content with quotes and newlines is JSON-escaped correctly (round-trip safety)

Combined substitutions (1 test):

• Both {env:} and {file:} work together in a single config parse

Critic review

Tests were reviewed by an automated critic agent. Key fixes applied:

• All error assertions use e.data.* (not e.properties.*) matching the actual NamedError shape
• Removed a test that claimed to test ~/ expansion but actually tested absolute paths
• Renamed "env substitution before JSONC parsing" to clarify it tests raw text injection behavior

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage via /test-discovery

How did you verify your code works?

bun test test/config/paths-parsetext.test.ts    # 16 pass, 0 fail

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

https://claude.ai/code/session_01Q1dSZU3cd7ojBSPJodbK1J

Summary by CodeRabbit

• Tests
  • Comprehensive test coverage for configuration file parsing functionality, validating JSON and JSONC format support with comments and trailing commas, environment variable substitution with fallback handling, file path resolution relative to configuration directories, detailed error reporting with line and column information, configurable missing file behavior, and complex multi-substitution scenarios.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

A new test suite for ConfigPaths.parseText has been added with 186 lines covering JSON/JSONC parsing, environment variable substitution using {env:VAR} syntax, file inclusion using {file:path} syntax, error handling for invalid configs and missing files, and combined substitutions.

Changes

Cohort / File(s)	Summary
Config Parser Tests packages/opencode/test/config/paths-parsetext.test.ts	New test suite validating ConfigPaths.parseText functionality including JSONC parsing with comments and trailing commas, {env:VAR} environment variable substitution, {file:path} file inclusion with relative path resolution and missing file handling, error reporting with line/column details, and combined substitution scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

contributor

Poem

🐰 With whiskers twitching, I test the path,
Parsing JSONC, counting the math,
Env vars and files substituted with care,
Errors caught early—bugs beware!
A rabbit's quest for code so fair! 🌿

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding comprehensive tests for ConfigPaths.parseText covering substitution and JSONC parsing functionality.
Description check	✅ Passed	The description is comprehensive and well-structured, covering all template sections: a detailed summary explaining what changed and why (emphasizing the critical nature of these untested functions), a test plan section documenting the 16 tests across four categories, and a checklist confirming tests were added and pass locally.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/test-config-parsetext-01Q1dSZU3cd7ojBSPJodbK1J

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/opencode/test/config/paths-parsetext.test.ts (1)

55-55: Use a non-secret-looking fixture value to avoid scanner noise.

"test-api-key-12345" at Line 55 matches common secret patterns and can trip secret-scanning tooling. A neutral value (for example, "test-value-12345") avoids false positives.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/config/paths-parsetext.test.ts` at line 55, The test
sets process.env[envKey] to a secret-looking string ("test-api-key-12345") which
can trigger secret scanners; change the fixture to a neutral value such as
"test-value-12345" (or similar non-key text) where process.env[envKey] is
assigned in the test (look for the process.env[envKey] = "...") so the test
still validates behavior without using a realistic API-key format.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/opencode/test/config/paths-parsetext.test.ts`:
- Around line 54-60: The afterEach currently unconditionally deletes
process.env[envKey], which can clobber a pre-existing value; change the setup in
the beforeEach/afterEach pair to save the previous value (const prev =
process.env[envKey]) in beforeEach (or an outer-scope variable) and in afterEach
restore it (if prev is undefined delete process.env[envKey] else set
process.env[envKey] = prev); apply this same snapshot-and-restore pattern for
any other env vars set in this test file (refer to envKey and the
beforeEach/afterEach functions to locate the places to change).

---

Nitpick comments:
In `@packages/opencode/test/config/paths-parsetext.test.ts`:
- Line 55: The test sets process.env[envKey] to a secret-looking string
("test-api-key-12345") which can trigger secret scanners; change the fixture to
a neutral value such as "test-value-12345" (or similar non-key text) where
process.env[envKey] is assigned in the test (look for the process.env[envKey] =
"...") so the test still validates behavior without using a realistic API-key
format.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f04a3c97-a9fb-47a3-8da2-49311b2652ee

📥 Commits

Reviewing files that changed from the base of the PR and between 8eb13d4 and f6dddc9.

📒 Files selected for processing (1)

• packages/opencode/test/config/paths-parsetext.test.ts

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Restore previous env value instead of always deleting it.

At Line 59, unconditional delete can clobber a pre-existing process env value and make tests order-dependent. Please snapshot and restore prior state (and apply the same pattern to other env vars set in this file).

Proposed fix

 describe("ConfigPaths.parseText: {env:VAR} substitution", () => {
   const envKey = "OPENCODE_TEST_PARSE_TEXT_KEY"
+  let previousEnvKey: string | undefined
 
   beforeEach(() => {
+    previousEnvKey = process.env[envKey]
     process.env[envKey] = "test-api-key-12345"
   })
 
   afterEach(() => {
-    delete process.env[envKey]
+    if (previousEnvKey === undefined) delete process.env[envKey]
+    else process.env[envKey] = previousEnvKey
   })

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	beforeEach(() => {
	process.env[envKey] = "test-api-key-12345"
	})
	afterEach(() => {
	delete process.env[envKey]
	})
	describe("ConfigPaths.parseText: {env:VAR} substitution", () => {
	const envKey = "OPENCODE_TEST_PARSE_TEXT_KEY"
	let previousEnvKey: string | undefined
	beforeEach(() => {
	previousEnvKey = process.env[envKey]
	process.env[envKey] = "test-api-key-12345"
	})
	afterEach(() => {
	if (previousEnvKey === undefined) delete process.env[envKey]
	else process.env[envKey] = previousEnvKey
	})

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/config/paths-parsetext.test.ts` around lines 54 - 60,
The afterEach currently unconditionally deletes process.env[envKey], which can
clobber a pre-existing value; change the setup in the beforeEach/afterEach pair
to save the previous value (const prev = process.env[envKey]) in beforeEach (or
an outer-scope variable) and in afterEach restore it (if prev is undefined
delete process.env[envKey] else set process.env[envKey] = prev); apply this same
snapshot-and-restore pattern for any other env vars set in this test file (refer
to envKey and the beforeEach/afterEach functions to locate the places to
change).

[anandgupta42]

Consolidated into #403