fix(engine): apply the JSON→YAML fallback in config-lint's canonical parser too - #7287
Conversation
…parser too
config-lint had two independent top-level-object parsers for the same manifest
text that disagreed on JSON/YAML fallback. `parseTopLevelObject` (used by
`unknownTopLevelWarnings`) retried with `parseYaml` when `JSON.parse` threw on a
`{`/`[`-prefixed text — YAML flow mappings can start that way while still being
valid manifest syntax. `parseCanonicalTopLevelObject` (used by
`recognizedFieldsFor`) did not: it caught the `JSON.parse` failure and returned
null → [], so `buildConfigLintReport` silently reported zero recognized fields
for a valid YAML-flow-mapping manifest that `unknownTopLevelWarnings` happily
parsed and warned about — the two functions produced inconsistent results for
the same input in the same report call.
De-duplicate the two into one shared `parseManifestTopLevelObject` used by both
callers, so recognizedFieldsFor and unknownTopLevelWarnings can never drift on
whether a given manifest text parses. Add a regression test asserting a valid
YAML flow mapping starting with `{` now yields non-empty recognized fields
consistent with the unknown-field warning for the same text.
Closes JSONbored#7244
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7287 +/- ##
==========================================
- Coverage 93.81% 93.81% -0.01%
==========================================
Files 704 704
Lines 69462 69454 -8
Branches 18896 18892 -4
==========================================
- Hits 65167 65159 -8
Misses 3302 3302
Partials 993 993
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-19 12:52:23 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
config-linthad two independent top-level-object parsers for the same manifest text that disagreed on JSON/YAML fallback.parseTopLevelObject(feedingunknownTopLevelWarnings) retried withparseYamlwhenJSON.parsethrew on a{/[-prefixed text — valid YAML flow mappings can start that way — butparseCanonicalTopLevelObject(feedingrecognizedFieldsFor) did not, sobuildConfigLintReportsilently reported zero recognized fields for a valid YAML-flow-mapping manifest thatunknownTopLevelWarningsparsed and warned about, producing internally inconsistent results for the identical input.parseManifestTopLevelObjectused by both callers, so the two can never drift on whether a manifest text parses. Added a regression test asserting a valid YAML flow mapping starting with{now yields non-empty recognized fields consistent with the unknown-field warning for the same text.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #7244.Validation
npm run typechecknpm run build --workspace @loopover/enginenpm run test --workspace @loopover/engine(589 passing)npm run test:engine-paritytest/unit/selfhost-config-lint.test.ts— 100% line and branch coverage onpackages/loopover-engine/src/config-lint.ts(43/43 branches)#7244regression test plus the existing flow-mapping testconfig-lint.tsand its unit test; no UI, API surface, MCP, docs, or dependency changes.Safety
hidden,secret) never leak into the result.UI Evidencerequired.Notes
"checks unknown fields in YAML flow mappings that start like JSON"test previously asserted the buggyrecognizedFields: []; it is updated to the now-correct["wantedPaths"], matching whatunknownTopLevelWarningsalready reported for the same text.Closes #7244