Reject null in vObjAny()

[monem (pucedoteth)]

Problem

TypeofValidator compares typeof content against the expected type name:

if (typeof content !== this.type) {
    return { content: undefined, error: { message: `Expected ${this.type}, but got ${typeof content}` } };
}

typeof null === 'object', and vObjAny() is new TypeofValidator('object'). So null passes validation:

vObjAny().validate(null)   ->  { content: null, error: undefined }

Two things that makes wrong:

• The declared type is ValidatorBase<object>, and TypeScript's object excludes null. A caller trusting the result gets a null the types say cannot be there.
• Its own getJSONSchema() returns { type: 'object' }, and in JSON Schema object excludes null too (null is a separate type). The validator contradicts the schema it advertises.

ObjValidator, in the same file, already guards exactly this:

if (typeof content !== 'object' || content === null) {
    return { content: undefined, error: { message: 'Expected object' } };
}

And the one in-tree caller works around it, re-checking after a successful validation — src/vs/platform/agentHost/node/claude/claudeSubagentResolver.ts:

const r = vObjAny().validate(input);
if (r.error || r.content === null || Array.isArray(r.content)) {
    return undefined;
}

That r.content === null branch only exists because vObjAny() lets null through.

Change

Reject null for the 'object' case, and report it as null rather than object so the message is accurate for every typeof validator. vString().validate(null) previously said "Expected string, but got object".

Before / after, running the real module:

input	before	after
vObjAny().validate(null)	OK content=null	Expected object, but got null
vObjAny().validate({a:1})	OK	OK (unchanged)
vObjAny().validate([1,2])	OK	OK (unchanged)
vObjAny().validate('str')	Expected object, but got string	unchanged
vString().validate(null)	Expected string, but got object	Expected string, but got null

Arrays still pass, as before — only null changes.

I left the r.content === null check in claudeSubagentResolver.ts alone. It is now redundant but still correct, and it guards untrusted SDK JSON; removing it felt like the wrong thing to bundle into this fix.

Tests

Added src/vs/base/test/common/validation.test.ts — the module had no tests. It covers null rejection, that objects and arrays still pass, non-object rejection, and the corrected null message on the other typeof validators.

Verification

I could not run the repo's test harness — I worked from a sparse checkout (src/vs/base, src/vs/platform, src/vs/editor/common) because a full clone kept timing out here, so there is no node_modules to build against. What I did run:

• Executed the real validation.ts module directly to produce the before/after table above, with the fix stashed and unstashed.
• tsc --strict --noEmit on validation.ts: no errors.
• The 8 errors tsc reports for validation.test.ts are all Cannot find name 'suite'/'test' and Cannot find module 'assert'. I checked this is environmental: running the identical command against the existing src/vs/base/test/common/numbers.test.ts produces 9 errors of the same class.

So the test file has not been executed by the real runner. Worth a CI run before merging, and I am happy to adjust if it disagrees with local conventions.

Note

TypeOfMap also has a null: null entry, but no validator is ever constructed with it, and it could not work as written for the same typeof reason. Left alone as out of scope — flagging it in case it is meant to be used.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the validation utilities to treat null as a distinct type in typeof-based validators (especially for 'object'), and adds regression tests to ensure null is rejected with a clear error message.

Changes:

• Adjusted TypeofValidator.validate to reject null when validating 'object' and to report null in error messages instead of 'object'.
• Added unit tests covering vObjAny() null rejection and vString()/vNumber() null error messaging.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/vs/base/test/common/validation.test.ts	Adds tests validating null handling and error messages for object and primitive validators.
src/vs/base/common/validation.ts	Fixes typeof-based validation to reject null for 'object' and improves error messaging to print null.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[monem (pucedoteth)]

Done in c4c8255.

The comment now states the typeof null === 'object' pitfall itself rather than naming vObjAny()/ObjValidator, and actualType is hoisted.

Hoisting it also let the second condition go: 'null' is never a TypeOfMap key, so actualType !== this.type rejects null on its own. Same behaviour for every type and identical messages -- just one comparison instead of two.

[monem (pucedoteth)]

Done in c4c8255.

The comment now states the typeof null === 'object' pitfall itself rather than naming vObjAny()/ObjValidator, and actualType is hoisted.

Hoisting it also let the second condition go: 'null' is never a TypeOfMap key, so actualType !== this.type rejects null on its own. Same behaviour for every type and identical messages -- just one comparison instead of two.

[monem (pucedoteth)]

@microsoft-github-policy-service agree