Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 10 additions & 19 deletions packages/loopover-engine/src/config-lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export function lintManifestText(text: string | null | undefined): SelfHostConfi
}

function recognizedFieldsFor(text: string | null | undefined): string[] {
const parsed = parseCanonicalTopLevelObject(text);
const parsed = parseManifestTopLevelObject(text);
if (parsed === null) return [];
return TOP_LEVEL_FIELDS.filter(
(field) => field !== "source" && Object.prototype.hasOwnProperty.call(parsed, field),
Expand All @@ -77,10 +77,7 @@ const RETIRED_FIELD_MIGRATION_WARNINGS: Record<string, string> = {
};

export function unknownTopLevelWarnings(text: string | null | undefined): string[] {
const raw = text ?? "";
const trimmed = raw.trim();
if (!trimmed || isOversize(raw)) return [];
const parsed = parseTopLevelObject(trimmed);
const parsed = parseManifestTopLevelObject(text);
if (parsed === null) return [];
const keys = Object.keys(parsed).filter((key) => !TOP_LEVEL_FIELD_SET.has(key));
// `hasOwnProperty.call`, NOT `key in`: a manifest field named like an Object.prototype member
Expand All @@ -96,30 +93,24 @@ export function unknownTopLevelWarnings(text: string | null | undefined): string
];
}

function parseCanonicalTopLevelObject(text: string | null | undefined): Record<string, unknown> | null {
// Single top-level-object parser shared by both `recognizedFieldsFor` and `unknownTopLevelWarnings` so the two
// can never disagree on whether a given manifest text parses. When the text looks like JSON (`{`/`[`) but
// `JSON.parse` throws, it retries with `parseYaml`: YAML flow mappings can start with "{" or "[" (e.g. unquoted
// keys) while still being valid manifest syntax, so a strict-JSON failure alone must not be treated as unparseable.
function parseManifestTopLevelObject(text: string | null | undefined): Record<string, unknown> | null {
const raw = text ?? "";
const trimmed = raw.trim();
if (!trimmed || isOversize(raw)) return null;
const looksLikeJson = trimmed.startsWith("{") || trimmed.startsWith("[");
try {
return topLevelObjectOrNull(looksLikeJson ? JSON.parse(trimmed) : parseYaml(trimmed));
} catch {
return null;
}
}

function parseTopLevelObject(text: string): Record<string, unknown> | null {
const looksLikeJson = text.startsWith("{") || text.startsWith("[");
if (looksLikeJson) {
try {
const parsed = JSON.parse(text);
return topLevelObjectOrNull(parsed);
return topLevelObjectOrNull(JSON.parse(trimmed));
} catch {
// YAML flow mappings can start with "{" or "[" while still being valid manifest syntax.
// Fall through to YAML: a `{`/`[` prefix can be a valid YAML flow mapping that is invalid strict JSON.
}
}
try {
return topLevelObjectOrNull(parseYaml(text));
return topLevelObjectOrNull(parseYaml(trimmed));
} catch {
return null;
}
Expand Down
21 changes: 19 additions & 2 deletions test/unit/selfhost-config-lint.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { lintManifestText } from "../../src/selfhost/config-lint";
import { lintManifestText, unknownTopLevelWarnings } from "../../src/selfhost/config-lint";
import { MAX_FOCUS_MANIFEST_BYTES } from "../../src/signals/focus-manifest";

describe("lintManifestText (#2079)", () => {
Expand Down Expand Up @@ -271,14 +271,31 @@ unknownSecretKey: super-secret-value
const result = lintManifestText("{wantedPaths: [src/], unknownSecretKey: secret}");

expect(result.ok).toBe(false);
expect(result.recognizedFields).toEqual([]);
// recognizedFieldsFor now applies the same JSON->YAML fallback as unknownTopLevelWarnings, so a flow-mapping
// manifest that is valid YAML but invalid strict JSON reports its real recognized fields instead of [].
expect(result.recognizedFields).toEqual(["wantedPaths"]);
expect(result.warnings).toEqual([
"Manifest content was not valid JSON; ignoring it and falling back to deterministic signals.",
"Manifest contains unknown top-level field: unknownSecretKey.",
]);
expect(JSON.stringify(result)).not.toContain("secret");
});

it("REGRESSION (#7244): recognizes fields in a YAML flow-mapping manifest starting with '{' consistently with unknown-field detection", () => {
// `{settings: ..., wantedPaths: ..., mysteryKey: ...}` is a valid YAML flow mapping but invalid strict JSON
// (unquoted keys). unknownTopLevelWarnings already fell back to YAML and saw the real fields; recognizedFieldsFor
// did NOT (it caught the JSON.parse failure and returned null -> []), so buildConfigLintReport silently reported
// zero recognized fields for a fully-parseable, valid manifest. Both must now agree the manifest parses.
const text = "{settings: {commentMode: all_prs}, wantedPaths: [src/], mysteryKey: hidden}";
const result = lintManifestText(text);

// recognizedFieldsFor (surfaced via result.recognizedFields) sees the real fields instead of silently empty.
expect(result.recognizedFields).toEqual(["wantedPaths", "settings"]);
// ...consistent with unknownTopLevelWarnings, which flags the unknown key on the SAME parsed object.
expect(unknownTopLevelWarnings(text)).toEqual(["Manifest contains unknown top-level field: mysteryKey."]);
expect(JSON.stringify(result)).not.toContain("hidden");
});

it("keeps known JSON manifests quiet and non-object JSON invalid", () => {
expect(lintManifestText(JSON.stringify({ gate: { enabled: true, checkMode: "required" } }))).toMatchObject({
ok: true,
Expand Down
Loading