Skip to content

test: validate shipped config templates#303

Merged
ndycode merged 1 commit intomainfrom
test/pr9-config-schema-validation
Mar 23, 2026
Merged

test: validate shipped config templates#303
ndycode merged 1 commit intomainfrom
test/pr9-config-schema-validation

Conversation

@ndycode
Copy link
Owner

@ndycode ndycode commented Mar 22, 2026

Summary

  • add focused coverage for config/schema/config.schema.json
  • validate the shipped templates (codex-modern.json, codex-legacy.json, minimal-codex.json) against the schema
  • add negative cases for missing required fields and wrong primitive types

Validation

  • npm run typecheck
  • npm run lint -- test/config-schema-templates.test.ts
  • npm run test -- test/config-schema-templates.test.ts
  • manual QA: direct Node verification of the three shipped templates against the schema

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 test/config-schema-templates.test.ts — three vitest cases that load the real config/schema/config.schema.json and validate the three shipped templates (codex-modern.json, codex-legacy.json, minimal-codex.json) against it, plus two negative cases for missing required fields and wrong primitive types.

the tests are logically correct and the custom validator correctly handles the schema's current shape (object, array, string). two non-blocking follow-up items:

  • process.cwd() path resolution — should be replaced with fileURLToPath(new URL("../..", import.meta.url)) for ESM-safe, shell-independent paths; on windows a mismatched cwd is a common trip wire.
  • missing boolean/number type branches — the custom validator silently returns zero errors for any schema field typed as "boolean" or "number"; not a problem today but creates a silent false-positive risk if the schema is tightened later (e.g., explicitly typing store: boolean).
  • no concurrency concerns — tests are read-only readFileSync calls with no shared mutable state.
  • no token or filesystem safety concerns beyond the cwd note above.

Confidence Score: 5/5

  • safe to merge — tests are correct and add real schema coverage; both flagged items are non-blocking P2 cleanups.
  • test logic is sound, assertions are correct (ordering in the third test is deterministic via V8's Object.entries insertion-order guarantee), and no production code is changed. the two P2 comments are follow-up improvements, not blockers.
  • no files require special attention.

Important Files Changed

Filename Overview
test/config-schema-templates.test.ts new test file adding three focused vitest cases: positive validation of all three shipped config templates, a required-field rejection, and a primitive-type rejection. logic is correct; two P2 concerns: process.cwd() portability and missing boolean/number type branches in the custom validator.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["test: config-schema-templates.test.ts"] --> B["readJson(config/schema/config.schema.json)"]
    B --> C{describe block}

    C --> T1["it: validates shipped templates"]
    T1 --> L1["readJson(codex-modern.json)"]
    T1 --> L2["readJson(codex-legacy.json)"]
    T1 --> L3["readJson(minimal-codex.json)"]
    L1 & L2 & L3 --> V1["validateAgainstSchema(payload, schema)"]
    V1 --> E1["expect errors == []"]

    C --> T2["it: rejects missing required fields"]
    T2 --> V2["validateAgainstSchema({plugin:[...]}, schema)"]
    V2 --> E2["expect errors toContain '$.provider is required'"]

    C --> T3["it: rejects wrong primitive types"]
    T3 --> V3["validateAgainstSchema({plugin:[123], provider:'openai'}, schema)"]
    V3 --> E3["expect errors == ['$.plugin[0] must be a string', '$.provider must be an object']"]

    subgraph validateAgainstSchema
        VA["schema.type == 'object'?"] -->|yes| VB["check required keys\niterate properties"]
        VA -->|"'array'"| VC["check Array.isArray\niterate items"]
        VA -->|"'string'"| VD["check typeof === 'string'"]
        VA -->|"other (boolean/number)"| VE["⚠️ silent return []"]
    end
Loading

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: test/config-schema-templates.test.ts
Line: 12

Comment:
**`process.cwd()` is fragile for path resolution in ESM**

this project is ESM-only (`"type": "module"`), so `__dirname` isn't available — but `import.meta.url` is. `process.cwd()` resolves to wherever the shell is when vitest is invoked; if someone runs a focused test from a subdirectory or a custom vitest `root` config is set, all `readJson` calls will throw `ENOENT` with zero indication of why.

use `fileURLToPath` instead so paths are always relative to this file:

```suggestion
const projectRoot = fileURLToPath(new URL("../..", import.meta.url));
```

you'd also need to add the import at the top:

```typescript
import { fileURLToPath } from "node:url";
```

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/config-schema-templates.test.ts
Line: 68-78

Comment:
**validator silently passes `boolean`/`number` types — schema drift risk**

the custom `validateAgainstSchema` handles `"object"`, `"array"`, and `"string"` but has no branch for `"boolean"` or `"number"`. the current `config.schema.json` doesn't use those primitive types at the top level, but provider option blobs like `"store": false` are already boolean values in the templates. if the schema is ever tightened to explicitly type those fields, any type mismatch will fall through to `return errors` silently (zero errors returned, test stays green when it shouldn't).

consider adding at minimum a `"boolean"` guard before the final `return errors`:

```typescript
if (schema.type === "boolean") {
    if (typeof value !== "boolean") {
        errors.push(`${pathLabel} must be a boolean`);
    }
    return errors;
}
```

alternatively, consider using a proper JSON Schema validator (`ajv`) to avoid maintaining a growing list of type branches.

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

Last reviewed commit: "test: validate shipp..."

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 22, 2026

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 57 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ee1a24d9-466d-4b12-a500-f6fdcf143283

📥 Commits

Reviewing files that changed from the base of the PR and between 1be5e95 and 8dfc2ff.

📒 Files selected for processing (1)
  • test/config-schema-templates.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/pr9-config-schema-validation
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch test/pr9-config-schema-validation

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.

@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.

@ndycode
Copy link
Owner Author

ndycode commented Mar 22, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ndycode ndycode merged commit 7378337 into main Mar 23, 2026
1 of 2 checks passed
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