Add schema validation for humans.txt.yaml - #2
Merged
Faisal M. AlOtaibi (hub966) merged 1 commit intoJul 25, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds explicit schema validation for humans.txt.yaml so malformed entries fail fast with clear, actionable error messages rather than silently passing through or causing downstream runtime issues.
Changes:
- Introduces
validateSchema()inaction.jsand runs it immediately after YAML parsing, emittingSchema error: …messages and setting exit code 1 on validation failures. - Extends
test.shwith negative-path validation cases intended to cover missingnameand unknown fields. - Documents the supported schema (fields, types, required/optional) in
README.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| action.js | Adds schema validation after YAML parse and fails fast with descriptive stderr output. |
| test.sh | Adds negative-path validation checks for invalid YAML inputs. |
| README.md | Documents the humans.txt.yaml schema and notes that unknown fields are rejected. |
Comments suppressed due to low confidence (2)
action.js:35
humans[i]is intended to be a YAML mapping, but arrays currently pass the check (typeof [] === 'object'). This can lead to confusing downstream errors (e.g., only a missing-name error) instead of a clear "must be a mapping" failure.
if (typeof human !== 'object' || human === null) {
errors.push(`${prefix} must be a mapping`)
return
}
test.sh:53
- This test also duplicates validation logic rather than verifying that
action.jsrejects unknown fields with a non-zero exit and the expected stderr prefix. It won't catch regressions in the actual CLI behavior.
node -e "
const fs = require('fs')
const yaml = require('yaml')
const data = yaml.parse('humans:\n - name: Test\n unknown_field: true\n')
const KNOWN_FIELDS = new Set(['name','alum','honorary_human'])
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+19
to
+22
| if (!data || typeof data !== 'object') { | ||
| errors.push("root must be a YAML mapping") | ||
| return errors | ||
| } |
Comment on lines
+23
to
+46
| node -e " | ||
| const fs = require('fs') | ||
| const yaml = require('yaml') | ||
| const data = yaml.parse(fs.readFileSync('/tmp/invalid-humans.yaml', 'utf8')) | ||
| // inline the validation function | ||
| const KNOWN_FIELDS = new Set(['name','alum','honorary_human']) | ||
| function validateSchema(data) { | ||
| const errors = [] | ||
| if (!data || typeof data !== 'object') { errors.push('root must be a YAML mapping'); return errors } | ||
| if (!Array.isArray(data.humans)) { errors.push(\"'humans' must be a list\"); return errors } | ||
| data.humans.forEach((human, i) => { | ||
| const prefix = 'humans[' + i + ']' | ||
| if (typeof human !== 'object' || human === null) { errors.push(prefix + ' must be a mapping'); return } | ||
| if (typeof human.name !== 'string' || human.name.trim() === '') errors.push(prefix + '.name must be a non-empty string') | ||
| if ('alum' in human && typeof human.alum !== 'boolean') errors.push(prefix + '.alum must be a boolean') | ||
| if ('honorary_human' in human && typeof human.honorary_human !== 'boolean') errors.push(prefix + '.honorary_human must be a boolean') | ||
| for (const key of Object.keys(human)) { if (!KNOWN_FIELDS.has(key)) errors.push(prefix + \" has unknown field '\" + key + \"'\") } | ||
| }) | ||
| return errors | ||
| } | ||
| const errs = validateSchema(data) | ||
| if (errs.length === 0) { console.error('Expected validation errors but got none'); process.exit(1) } | ||
| console.log('Validation correctly rejected invalid data:', errs) | ||
| " |
Collaborator
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
humans.txt.yamlhad no enforcement of its data model — malformed entries (missing names, wrong types, unknown fields) would silently pass through or cause cryptic runtime errors.Changes
action.js— addsvalidateSchema()called immediately after YAML parse; fails fast with descriptivestderrmessages and exit code 1 on:namealum/honorary_humanpresent but not booleantest.sh— adds two negative-path cases: missingnameand unknown field, both asserting errors are producedREADME.md— documents the schema (field names, types, required/optional)Example error output for an invalid entry: