fix(engine): apply the JSON→YAML fallback in config-lint's canonical 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 #7244

Author: bitfathers94

packages/loopover-engine/src/config-lint.ts

@@ -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),
@@ -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
@@ -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;
   }

test/unit/selfhost-config-lint.test.ts

@@ -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)", () => {
@@ -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,