Allow comments in providers.json, and make robust to unknown fields - #53
Merged
Conversation
Whole-source failures (user-file JSONC syntax errors, non-ENOENT read
errors, env-fragment parse/validation failures) now produce
error-severity issues instead of warnings; per-key salvage drops stay
warnings. parseJsonc reports syntax errors as 1-based line/column
(computed from the text, since jsonc-parser only carries offsets) on
both the tolerant and strict paths. Whole-source failure messages are
reworded to be source-agnostic ("Invalid JSONC: ...", "Could not read
the file: ...") so hosts can compose a "Failed to load <path>" prefix
from source identity. Hosts surface only error-severity issues in the
UI; warning-severity drops are log-only because providers.json is
shared across consumers with different provider vocabularies.
Review follow-up: the host error-only UI filter relied on a severity/scope contract the producers did not consistently satisfy. Whole-file salvage degrades (non-object root/providers, unsupported version), schema-invalid env fragments, and malformed legacy POSITRON_ENFORCED_SETTINGS all discard their entire source but arrived as warnings (hidden by the filter) or with per-key paths (misrendered as per-key drops). Whole-source failures now uniformly emit error-severity issues with an empty path; offending key paths stay in the message prose. Also fixes lineColumnAt to treat CR and CRLF as line breaks, matching jsonc-parser.
Review follow-up: recoverValidStack drops an entire overlay at merge time but emitted a warning with a Zod path, invisible under the hosts' error-only UI filters. The whole-source contract (error severity, empty path, detail in message prose) was recreated by open-coded objects and path-taking local helpers at four sites, which is why producers kept missing it. Add wholeSourceIssue/sourcedWholeSourceIssue in config-issue.ts, convert all five call sites (file read/parse, env fragments, salvage degrades and seal failure, legacy enforced settings, resolver overlay drops), and delete the local helpers so the error-with-a-path combination is unrepresentable.
ConfigIssue is now a discriminated union: the error branch (whole-source failures) types its path as readonly [], so an error with a non-empty path is unrepresentable at the type level rather than by convention. SourcedConfigIssue becomes an intersection over the union.
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
1 participant
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.
Allow comments in
providers.json, and make robust to unknown fieldsBefore this change,
providers.jsonwas handled pretty strictly: if there were any comments, or any fields that weren't specified in schema, then the code would return an empty{}object to represent the configuration.This PR allows
providers.jsonreads to preserve valid provider settings when the file contains JSONC syntax or an unknown or invalid provider block. Reads now return structured, sourced issue snapshots for anything they skip, while mutations remain strict so a programmatic rewrite can never silently delete configuration it does not understand.Changes
providers, unsupportedversion), an invalid env fragment, or malformed legacy enforced settings — as an error-severity issue with an empty path, while per-key salvage drops remain warnings scoped to the dropped key. Hosts can therefore surface only whole-source failures in the UI and keep validation drops log-only.