diff --git a/src/main/yaml-path.ts b/src/main/yaml-path.ts index 9951860fa..6ec104b00 100644 --- a/src/main/yaml-path.ts +++ b/src/main/yaml-path.ts @@ -27,47 +27,54 @@ export function getYamlPath(content: string, dottedKey: string): string | null { if (parts.length === 0) return null; const lines = content.split(/\r?\n/); - // Stack of (indent, key) frames describing the parent path being walked. - // The current frame is the deepest one we've descended into; siblings or - // dedents pop it. - const stack: { indent: number; key: string }[] = []; - let pathIdx = 0; - for (const raw of lines) { - const trimmed = raw.trimStart(); - if (!trimmed || trimmed.startsWith("#")) continue; + // Walk the path one segment at a time. `searchStart` is the first line to + // scan for the current segment; `parentIndent` bounds the parent's block — + // children live at indent strictly greater than it. The first segment uses + // parentIndent = -1, so only column-0 keys match (a flat/single-segment key + // is pinned to the top level and never resolves a nested occurrence). + let searchStart = 0; + let parentIndent = -1; - const indent = raw.length - trimmed.length; - // Pop stack frames whose indent is >= the current line's indent — those - // are siblings/cousins of the current node, not parents. - while (stack.length > 0 && stack[stack.length - 1].indent >= indent) { - stack.pop(); - } - // If we've already drilled into a deeper level than where the current - // pathIdx parent lives, the dotted path is broken (we walked past it - // without finding the next part), so reset pathIdx to the depth we are - // actually at — i.e. number of parts already matched in stack. - pathIdx = stack.length; + for (let p = 0; p < parts.length; p++) { + const isLeaf = p === parts.length - 1; + // The shallowest non-blank line inside the block is the direct-child + // depth. Lines deeper than that are grandchildren and are skipped, so a + // segment only matches a *direct* child of its parent. + let directChildIndent: number | null = null; + let descendInto = -1; + + let i = searchStart; + for (; i < lines.length; i++) { + const raw = lines[i]; + const trimmed = raw.trimStart(); + if (!trimmed || trimmed.startsWith("#")) continue; + + const indent = raw.length - trimmed.length; + // A non-blank line at or shallower than the parent closes the block. + if (indent <= parentIndent) break; - const colon = trimmed.indexOf(":"); - if (colon < 0) continue; - const rawKey = trimmed.slice(0, colon).trim(); - if (!rawKey) continue; - // Quoted keys aren't used in Hermes config but strip the wrapping just in - // case so `"memory": ...` would still match. - const key = stripQuotes(rawKey); - const remainder = trimmed.slice(colon + 1); + if (directChildIndent === null) directChildIndent = indent; + if (indent !== directChildIndent) continue; // grandchild — skip - if (pathIdx < parts.length && key === parts[pathIdx]) { - const isLeaf = pathIdx === parts.length - 1; - if (isLeaf) { - return parseScalar(remainder); - } - // Intermediate key — push onto the stack and look for the next part - // among its children. - stack.push({ indent, key }); - pathIdx = stack.length; + const colon = trimmed.indexOf(":"); + if (colon < 0) continue; + const rawKey = trimmed.slice(0, colon).trim(); + if (!rawKey) continue; + // Quoted keys aren't used in Hermes config but strip the wrapping just + // in case so `"memory": ...` would still match. + if (stripQuotes(rawKey) !== parts[p]) continue; + + if (isLeaf) return parseScalar(trimmed.slice(colon + 1)); + descendInto = i; + break; } + + // Leaf not found among the direct children, or an intermediate segment + // is missing → the path doesn't resolve. + if (isLeaf || descendInto < 0) return null; + searchStart = descendInto + 1; + parentIndent = directChildIndent as number; } return null; } diff --git a/tests/config-value-paths.test.ts b/tests/config-value-paths.test.ts index ca26b66b7..99a8216d6 100644 --- a/tests/config-value-paths.test.ts +++ b/tests/config-value-paths.test.ts @@ -115,11 +115,9 @@ describe("getConfigValue — dotted paths (issue #247)", () => { expect(getConfigValue("agent.service_tier")).toBeNull(); }); - // Skipped: getYamlPath (introduced by #243) is currently permissive on - // grandchildren and flat-key column-0 enforcement. The strictness - // these cases document is desired but not yet present; tracked as a - // follow-up against `yaml-path.ts`. - it.skip("ignores grandchildren — agent.service_tier matches only direct child", async () => { + // getYamlPath enforces direct-child depth: a dotted path's leaf must be a + // direct child of its parent block, not a grandchild nested deeper down. + it("ignores grandchildren — agent.service_tier matches only direct child", async () => { writeFileSync( join(TEST_DIR, "config.yaml"), [ @@ -150,10 +148,9 @@ describe("getConfigValue — flat keys pinned to top level", () => { expect(getConfigValue("timezone")).toBe("America/New_York"); }); - // Skipped: see note on the grandchildren test above — `getYamlPath` - // currently falls through to nested matches when called with a flat - // key. Desired behavior is column-0 enforcement; tracked separately. - it.skip("does NOT match a nested occurrence when called with a flat key", async () => { + // Flat (single-segment) keys are pinned to column 0, so a nested + // occurrence of the same name must not be returned. + it("does NOT match a nested occurrence when called with a flat key", async () => { writeFileSync( join(TEST_DIR, "config.yaml"), ["agent:", " service_tier: fast", " max_turns: 60", ""].join("\n"), @@ -163,7 +160,7 @@ describe("getConfigValue — flat keys pinned to top level", () => { expect(getConfigValue("service_tier")).toBeNull(); }); - it.skip("does NOT pick the first nested occurrence across siblings", async () => { + it("does NOT pick the first nested occurrence across siblings", async () => { writeFileSync( join(TEST_DIR, "config.yaml"), [