From c6e65ae55bbc04e4378d5f53f914eb6f399a4740 Mon Sep 17 00:00:00 2001
From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com>
Date: Wed, 11 Nov 2026 23:07:50 -0500
Subject: [PATCH 1/5] fix(engine): warn of a _layering or _remove that nothing
 reads

Two directive spellings ran with nothing said. A single document whose top-level _layering is outside "replace", "shallow", "deep" completed silently, because nothing folds a single document and so nothing read the value, while mode: render refused the same file as a layer. A _remove on an entry of a list the registry does not key (a ruleset's bypass_actors, a rule's parameter lists, check_suite_preferences.auto_trigger_checks, any list under a passthrough key) removed nothing and passed through to GitHub as a field named _remove, from a single document and from a layer alike.

Both runs complete today, so neither is refused before the next major: each now draws a warning naming the source and the path, and the two accepting paths in validateSettingsDoc carry COMPAT(v4) markers naming the refusal that replaces the warning.

separateRemovals, the one walk over a document's removals, now also gathers every _remove on an entry no list keys: below the mapping sections, and below each kept keyed entry outside its declared nested lists. Each path is spelled from its steps as validation spells an issue's, so a key that only looks like a list step is quoted and the way back to the document as written never renumbers through it. A layer is judged as its standalone view, so validateLayer hands validateSettingsDoc the view's way back and a layer's warning names the entry by its index in the file.

The wording lives in engine/directives.ts beside the directive keys, and admit.ts now reads the one isLayering predicate from there.
---
 docs/operate/layering.md        |   2 +-
 src/engine/admit.ts             |  13 +-
 src/engine/directives.ts        |  58 +++++++-
 src/engine/orchestrate.ts       |  13 ++
 src/engine/separate-removals.ts | 108 ++++++++++++---
 src/flows/layers.ts             |   4 +-
 test/engine/orchestrate.test.ts | 238 ++++++++++++++++++++++++++++++++
 test/flows/layers.test.ts       |  33 +++++
 8 files changed, 437 insertions(+), 32 deletions(-)

diff --git a/docs/operate/layering.md b/docs/operate/layering.md
index 40e843994..aa3809fff 100644
--- a/docs/operate/layering.md
+++ b/docs/operate/layering.md
@@ -76,7 +76,7 @@ Beside these, `_remove: true` on a keyed entry drops the lower entry under that
 
 Any other underscore key at the top level or on a wrapper fails validation with an error naming which directive belongs where (a note belongs in a YAML comment). A misspelled directive can therefore never pass as a private note and quietly merge a layer meant to replace.
 
-Inside a keyed entry, an underscore key is a field like any other: a closed entry shape refuses it as unrecognized, and an open one keeps it, as it keeps every unknown key.
+Inside a keyed entry, an underscore key is a field like any other: a closed entry shape refuses it as unrecognized, and an open one keeps it, as it keeps every unknown key. A `_remove` on an entry of a list that is not keyed (a ruleset's `bypass_actors`) is such a field: it removes nothing, and the run warns, naming its path.
 
 ## The rules
 
diff --git a/src/engine/admit.ts b/src/engine/admit.ts
index 2d40ded51..51c544cdb 100644
--- a/src/engine/admit.ts
+++ b/src/engine/admit.ts
@@ -16,13 +16,16 @@ import {
   UNDECLARED_POLICIES,
 } from "../sections/shared/schema-helpers.js";
 import type { DistributiveOmit, UndeclaredPolicy } from "../types.js";
-import { KNOBBED, LAYERING_KEY, nestedForm, REMOVE_KEY, UNDECLARED_KEY } from "./directives.js";
+import {
+  isLayering,
+  KNOBBED,
+  LAYERING_KEY,
+  nestedForm,
+  REMOVE_KEY,
+  UNDECLARED_KEY,
+} from "./directives.js";
 import type { Layer } from "./layers.js";
 
-function isLayering(value: unknown): value is Layering {
-  return LAYERINGS.some((layering) => layering === value);
-}
-
 function isUndeclaredPolicy(value: unknown): value is UndeclaredPolicy {
   return UNDECLARED_POLICIES.some((policy) => policy === value);
 }
diff --git a/src/engine/directives.ts b/src/engine/directives.ts
index aad783971..b93b72149 100644
--- a/src/engine/directives.ts
+++ b/src/engine/directives.ts
@@ -1,11 +1,13 @@
 /**
  * The three directive keys a layer may write and the two forms of a keyed list they ride on, the one spelling every
- * stage of the fold reads (admit.ts, undeclared.ts, separate-removals.ts, layers.ts). Which location takes which key
- * is docs/operate/layering.md's.
+ * stage of the fold reads (admit.ts, undeclared.ts, separate-removals.ts, layers.ts), and what a run says of a
+ * directive written where nothing reads it. Which location takes which key is docs/operate/layering.md's.
  */
 
-import { isPlainObject } from "../plain-data.js";
+import { describeKind, isPlainObject } from "../plain-data.js";
 import { UNDECLARED_POLICY_SECTIONS } from "../schema.js";
+import { LAYERINGS, type Layering } from "../sections/shared/schema-helpers.js";
+import { quote } from "../text.js";
 
 /** The merge directive's key: at a file's top level, or on a list section's wrapper. */
 export const LAYERING_KEY = "_layering";
@@ -35,3 +37,53 @@ export function nestedForm(value: unknown): NestedForm | null {
   }
   return null;
 }
+
+/** Whether a value is one the `_layering` directive takes. */
+export function isLayering(value: unknown): value is Layering {
+  return LAYERINGS.some((layering) => layering === value);
+}
+
+/** The directives a document writes where nothing reads them, as validateSettingsDoc (orchestrate.ts) finds them. */
+export interface UnreadDirectives {
+  /** The document's top-level `_layering` as written, undefined when it sets none. */
+  readonly layering: unknown;
+
+  /** The path of each `_remove` on an entry no list keys (`SeparatedRemovals.unkeyed`). */
+  readonly unkeyed: readonly string[];
+
+  /** A layer's way back from its standalone view to the file as written, applied to each path; a document judged as written passes none. */
+  readonly asWritten: ((path: string) => string) | undefined;
+}
+
+/**
+ * The warning for a top-level `_layering` outside the directive's values, or null where the key is absent or legal.
+ * Nothing folds a single document, so the value is never read there; the fold refuses it in a layer (admit.ts), whose
+ * standalone view never carries the key this far. The value stays out of the text, as it does in that refusal.
+ */
+function unreadLayeringNote(source: string, value: unknown): string | null {
+  if (value === undefined || isLayering(value)) {
+    return null;
+  }
+  const got = `${describeKind(value)}${typeof value === "string" ? " that is none of them" : ""}`;
+  return (
+    `${source}: ${LAYERING_KEY} must be one of ${LAYERINGS.map(quote).join(", ")}; got ${got}. ` +
+    "A single document is not folded, so the directive is not read and the run goes on; mode: render refuses this " +
+    "file as a layer. Write one of the values, or remove the key; the next major version refuses it here too"
+  );
+}
+
+/** The warning for a `_remove` on an entry no list keys, by the marker's path (`rulesets[0].bypass_actors[0]._remove`). */
+export function unkeyedRemovalNote(source: string, path: string): string {
+  return (
+    `${source}: ${path}: ${REMOVE_KEY} belongs on an entry of a keyed list, and this list is not keyed, so it ` +
+    "removes nothing and passes through as a field of the entry. Remove the key; the next major version refuses it"
+  );
+}
+
+/** One warning per directive the document writes where nothing reads it, each naming `source`: the `_layering` first, then every marker in the walk's order. */
+export function unreadDirectiveNotes(source: string, unread: UnreadDirectives): string[] {
+  const asWritten = unread.asWritten ?? ((path: string) => path);
+  const notes = unread.unkeyed.map((path) => unkeyedRemovalNote(source, asWritten(path)));
+  const layering = unreadLayeringNote(source, unread.layering);
+  return layering === null ? notes : [layering, ...notes];
+}
diff --git a/src/engine/orchestrate.ts b/src/engine/orchestrate.ts
index c7d9ba273..58e5e78da 100644
--- a/src/engine/orchestrate.ts
+++ b/src/engine/orchestrate.ts
@@ -34,6 +34,7 @@ import {
 import { SECTIONS } from "../sections/registry.js";
 import { agree, countNoun } from "../text.js";
 import type { MustBeNever, UndeclaredPolicy } from "../types.js";
+import { unreadDirectiveNotes } from "./directives.js";
 import { executePlan } from "./execute.js";
 import { UNDECLARED_POLICIES } from "./layers.js";
 import type { RunOutcome } from "./outcome.js";
@@ -109,6 +110,9 @@ export interface ValidateOptions {
 
   /** Who authored the document, "operator" when omitted; the multi-repo flow passes "target" for a target's own settings.yml, so a secret reference in it is refused here, with the rest of the document's problems. */
   readonly secretSource?: SettingsSource | undefined;
+
+  /** A layer's way back from its standalone view to the file as written (`StandaloneView.asWritten`), for the path a warning names; a document judged as written passes none. */
+  readonly asWritten?: ((path: string) => string) | undefined;
 }
 
 /** What one section's run ends in: the drift it found (check) or the changes it made (apply), with its notes. */
@@ -205,6 +209,15 @@ export function validateSettingsDoc(
   // key, so judged as written it would fail every other required field, and a closed shape would name the marker again.
   const removals = separateRemovals(settings);
   issues.push(...removals.sites.map(singleDocumentRemovalIssue));
+  // COMPAT(v4): a single document whose top-level _layering is outside its values still runs, under a warning; delete the warning and refuse the value as _undeclared's is refused above
+  // COMPAT(v4): a _remove on an entry no list keys still runs and passes through as a field, under a warning; delete the warning and refuse each path with the removal sites above
+  unreadDirectiveNotes(sourceLabel, {
+    layering: (settings as Record<string, unknown>)._layering,
+    unkeyed: removals.unkeyed,
+    asWritten: options.asWritten,
+  }).forEach((note) => {
+    io.annotate("warning", note);
+  });
   const shapes = validateSectionShapes(
     (removals.sites.length === 0 ? settings : removals.rest) as Record<string, unknown>,
     sourceLabel,
diff --git a/src/engine/separate-removals.ts b/src/engine/separate-removals.ts
index 262b930e3..7a5d77b72 100644
--- a/src/engine/separate-removals.ts
+++ b/src/engine/separate-removals.ts
@@ -1,26 +1,44 @@
 /**
- * A document's removal entries set apart from it, and the way back from an issue over the rest to the document as
- * written. validateSettingsDoc (orchestrate.ts) refuses a single document's sites and judges the rest; the fold
- * (layers.ts) builds each layer's standalone view on it.
+ * A document's removal entries set apart from it, the markers no list keys beside them, and the way back from an
+ * issue over the rest to the document as written. validateSettingsDoc (orchestrate.ts) refuses a single document's
+ * sites and judges the rest; the fold (layers.ts) builds each layer's standalone view on it.
  */
 
+import { z } from "zod";
 import { isPlainObject } from "../plain-data.js";
-import { LIST_SECTIONS } from "../schema.js";
+import { LIST_SECTIONS, SECTION_KEYS } from "../schema.js";
 import type { KeyedListLayering } from "../sections/contract/keyed-list.js";
 import { listLayering } from "../sections/registry.js";
 import { nestedForm, REMOVE_KEY } from "./directives.js";
 
+/** The sections the keyed walk never enters: every list below one is a list nothing keys. */
+const MAPPING_SECTIONS = SECTION_KEYS.filter(
+  (key) => !(LIST_SECTIONS as readonly string[]).includes(key),
+);
+
 /** Whether an entry carries the marker at all, whatever its value: what the standalone view drops and a single document refuses. */
 function carriesRemoval(entry: Readonly<Record<string, unknown>>): boolean {
   return entry[REMOVE_KEY] !== undefined;
 }
 
+/** A path as its steps from the section key down, a list index a number; `z.core.toDotPath` spells it as validation spells an issue's, a key that is no plain name quoted. */
+type Steps = readonly (string | number)[];
+
 /**
  * Per list as validation spells it over the document minus its removals (`labels`, `labels.entries`,
  * `environments[0].variables`), the source index of each kept entry; only a list a removal shifted is recorded.
  */
 type SourceIndices = Map<string, readonly number[]>;
 
+/** What one walk of a document gathers beside its partitions. */
+interface Gathered {
+  /** Where each kept entry of a shifted list sat in the document as written. */
+  readonly sources: SourceIndices;
+
+  /** Each marker on an entry no list keys, by its path as validation spells one over the document minus its removals. */
+  readonly unkeyed: string[];
+}
+
 /** A keyed list's removals set apart from it, its entries' nested lists likewise, in the form the layer wrote them. */
 interface Partition {
   /** The entries minus the removals, each entry's declared nested lists partitioned in turn. */
@@ -30,16 +48,45 @@ interface Partition {
   readonly sites: string[];
 }
 
-/** A kept entry with each declared nested list partitioned in turn, and the sites of the removals below it. */
+/**
+ * The markers below a value no keyed list reaches: a `_remove` on a list's entry there removes nothing (the fold
+ * replaces such a list whole), so the entry stays and each marker is gathered by its path. `above` holds the
+ * containers on the way down, so a YAML alias cycle ends the descent.
+ */
+function gatherUnkeyed(value: unknown, path: Steps, unkeyed: string[], above: Set<object>): void {
+  if ((!Array.isArray(value) && !isPlainObject(value)) || above.has(value)) {
+    return;
+  }
+
+  above.add(value);
+  if (Array.isArray(value)) {
+    // Indexed, as in partitionRemovals: the walk runs before the plainness check.
+    for (let index = 0; index < value.length; index += 1) {
+      const item: unknown = value[index];
+      if (isPlainObject(item) && carriesRemoval(item)) {
+        unkeyed.push(z.core.toDotPath([...path, index, REMOVE_KEY]));
+      }
+      gatherUnkeyed(item, [...path, index], unkeyed, above);
+    }
+  } else {
+    for (const [field, child] of Object.entries(value)) {
+      gatherUnkeyed(child, [...path, field], unkeyed, above);
+    }
+  }
+  above.delete(value);
+}
+
+/** A kept entry with each declared nested list partitioned in turn, and the sites of the removals below it; its other fields are walked for the markers no list keys. */
 function partitionNestedLists(
   entry: Readonly<Record<string, unknown>>,
   keyed: KeyedListLayering,
   site: string,
-  viewSite: string,
-  sources: SourceIndices,
+  viewSite: Steps,
+  gathered: Gathered,
 ): { entry: Record<string, unknown>; sites: string[] } {
   const out: Record<string, unknown> = { ...entry };
   const sites: string[] = [];
+  const partitioned = new Set<string>();
   for (const [field, nested] of Object.entries(keyed.nested ?? {})) {
     const form = nestedForm(entry[field]);
     if (form === null) {
@@ -50,25 +97,32 @@ function partitionNestedLists(
       form.entries,
       nested,
       `${site}.${field}`,
-      form.knobs === null ? `${viewSite}.${field}` : `${viewSite}.${field}.entries`,
-      sources,
+      form.knobs === null ? [...viewSite, field] : [...viewSite, field, "entries"],
+      gathered,
     );
     sites.push(...below.sites);
     out[field] = form.knobs === null ? below.kept : { ...form.knobs, entries: below.kept };
+    partitioned.add(field);
+  }
+
+  for (const [field, value] of Object.entries(entry)) {
+    if (!partitioned.has(field)) {
+      gatherUnkeyed(value, [...viewSite, field], gathered.unkeyed, new Set());
+    }
   }
   return { entry: out, sites };
 }
 
 /**
- * `path` names the list as the sites do, by the document's own indices; `viewPath` as validation spells the kept
- * entries (`labels.entries`, `environments[1].variables`), the key under which `sources` records their origins.
+ * `path` names the list as the sites do, by the document's own indices; `viewPath` as validation reaches the kept
+ * entries (`labels.entries`, `environments[1].variables`), the key under which `gathered` records their origins.
  */
 function partitionRemovals(
   entries: readonly unknown[],
   keyed: KeyedListLayering,
   path: string,
-  viewPath: string,
-  sources: SourceIndices,
+  viewPath: Steps,
+  gathered: Gathered,
 ): Partition {
   const kept: unknown[] = [];
   const sites: string[] = [];
@@ -89,15 +143,15 @@ function partitionRemovals(
       continue;
     }
 
-    const viewSite = `${viewPath}[${kept.length}]`;
+    const viewSite = [...viewPath, kept.length];
     origins.push(index);
-    const below = partitionNestedLists(entry, keyed, site, viewSite, sources);
+    const below = partitionNestedLists(entry, keyed, site, viewSite, gathered);
     sites.push(...below.sites);
     kept.push(below.entry);
   }
 
   if (origins.some((source, index) => source !== index)) {
-    sources.set(viewPath, origins);
+    gathered.sources.set(z.core.toDotPath(viewPath), origins);
   }
   return { kept, sites };
 }
@@ -141,22 +195,26 @@ export interface SeparatedRemovals {
   /** Each removal's site (`labels[0]._remove`): list sections in LIST_SECTIONS order, each list as written, nested lists under their entry. */
   readonly sites: readonly string[];
 
+  /** Each `_remove` on an entry no list keys, which stays in `rest` with its marker: by its path as validation spells one (`rulesets.entries[0].bypass_actors[0]._remove`), indices as written, the list sections first. */
+  readonly unkeyed: readonly string[];
+
   /** A validation issue over `rest`, its leading path renumbered to the document as written: a removal dropped from the list shifts every entry after it, so `labels[0].color` in `rest` is the reader's `labels[1].color`. */
   readonly asWritten: (issue: string) => string;
 }
 
 /**
- * Every list section partitioned by the walk the fold uses. A single document has no lower layer to remove from, so
+ * Every section walked once: a list section partitioned by the walk the fold uses, the rest of the document searched
+ * for the markers that walk does not reach. A single document has no lower layer to remove from, so
  * validateSettingsDoc refuses the sites and judges `rest`; a layer of a fold reaches it as its standalone view,
  * `rest` minus the `_layering` directives. Either way what is said about `rest` is said of the document as written.
  */
 export function separateRemovals(doc: unknown): SeparatedRemovals {
   if (!isPlainObject(doc)) {
-    return { rest: doc, sites: [], asWritten: (issue) => issue };
+    return { rest: doc, sites: [], unkeyed: [], asWritten: (issue) => issue };
   }
   const rest: Record<string, unknown> = { ...doc };
   const sites: string[] = [];
-  const sources: SourceIndices = new Map();
+  const gathered: Gathered = { sources: new Map(), unkeyed: [] };
   for (const key of LIST_SECTIONS) {
     const form = nestedForm(doc[key]);
     if (form === null) {
@@ -166,11 +224,17 @@ export function separateRemovals(doc: unknown): SeparatedRemovals {
       form.entries,
       listLayering(key),
       key,
-      form.knobs === null ? key : `${key}.entries`,
-      sources,
+      form.knobs === null ? [key] : [key, "entries"],
+      gathered,
     );
     sites.push(...below.sites);
     rest[key] = form.knobs === null ? below.kept : { ...form.knobs, entries: below.kept };
   }
-  return { rest, sites, asWritten: renumbering(sources) };
+
+  for (const key of MAPPING_SECTIONS) {
+    gatherUnkeyed(doc[key], [key], gathered.unkeyed, new Set());
+  }
+
+  const asWritten = renumbering(gathered.sources);
+  return { rest, sites, unkeyed: gathered.unkeyed.map(asWritten), asWritten };
 }
diff --git a/src/flows/layers.ts b/src/flows/layers.ts
index 647f12b49..4112513a7 100644
--- a/src/flows/layers.ts
+++ b/src/flows/layers.ts
@@ -47,7 +47,9 @@ export interface FoldedLayers {
  */
 function validateLayer(layer: Layer, io: Io): Result<ValidatedSettings, SettingsProblem> {
   const view = standaloneView(layer.doc);
-  return validateSettingsDoc(view.doc, layer.name, EVERY_SECTION, io).mapErr((problem) =>
+  return validateSettingsDoc(view.doc, layer.name, EVERY_SECTION, io, {
+    asWritten: view.asWritten,
+  }).mapErr((problem) =>
     problem.code === "settings-malformed-sections"
       ? { ...problem, issues: problem.issues.map(view.asWritten) }
       : problem,
diff --git a/test/engine/orchestrate.test.ts b/test/engine/orchestrate.test.ts
index 2f207a6ca..d8680e842 100644
--- a/test/engine/orchestrate.test.ts
+++ b/test/engine/orchestrate.test.ts
@@ -1,6 +1,8 @@
 import { afterEach, describe, expect, spyOn, test } from "bun:test";
 import { err, ok } from "neverthrow";
 
+import { unkeyedRemovalNote } from "../../src/engine/directives.js";
+import { standaloneView } from "../../src/engine/layers.js";
 import {
   preflightProbe,
   type RepoRunOptions,
@@ -297,6 +299,31 @@ describe("validateSettingsDoc secret references", () => {
 });
 
 describe("validateSettingsDoc", () => {
+  const TEAM_ACTOR = { actor_id: 1, actor_type: "Team", bypass_mode: "always" };
+
+  /** One ruleset holding the given fields, as a document. */
+  const ruleset = (fields: Record<string, unknown>) => ({
+    rulesets: [{ name: "r", target: "branch", enforcement: "active", ...fields }],
+  });
+
+  /** One ruleset whose pull_request rule holds the given parameters beside the required ones. */
+  const pullRequestRuleset = (parameters: Record<string, unknown>) =>
+    ruleset({
+      rules: [
+        {
+          type: "pull_request",
+          parameters: {
+            dismiss_stale_reviews_on_push: false,
+            require_code_owner_review: false,
+            require_last_push_approval: false,
+            required_approving_review_count: 1,
+            required_review_thread_resolution: false,
+            ...parameters,
+          },
+        },
+      ],
+    });
+
   test("unknown top-level keys are one collected line naming the known sections", () => {
     const { io } = captureIo();
     expect(validateSettingsDoc({ labls: [] }, "repos/x.yml", SectionSelection.ALL, io)).toEqual(
@@ -341,6 +368,29 @@ describe("validateSettingsDoc", () => {
     ).toBe(true);
   });
 
+  // Nothing folds a single document, so nothing read the value: `_layering: union` ran silently, while mode: render refused the same file as a layer.
+  test.each<[what: string, value: unknown, got: string]>([
+    ["a string outside its values", "union", "a string that is none of them"],
+    ["null", null, "null"],
+  ])(
+    "a single document's top-level _layering that is %s draws one warning naming the values, and the document still validates",
+    (_what, value, got) => {
+      const { io, annotations } = captureIo();
+      const doc = { _layering: value, labels: [{ name: "bug" }] };
+      expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io).isOk()).toBe(true);
+      expect(annotations).toEqual([
+        `warning: s.yml: _layering must be one of "replace", "shallow", "deep"; got ${got}. A single document is not folded, so the directive is not read and the run goes on; mode: render refuses this file as a layer. Write one of the values, or remove the key; the next major version refuses it here too`,
+      ]);
+    },
+  );
+
+  test("control: a top-level _layering of the set is the file's directive as a layer, so a single document carries it without a warning", () => {
+    const { io, annotations } = captureIo();
+    const doc = { _layering: "shallow", labels: [{ name: "bug" }] };
+    expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io).isOk()).toBe(true);
+    expect(annotations).toEqual([]);
+  });
+
   // Each of these once stopped the run alone, so a file with all three took three runs to fix.
   test("an unknown directive, an unknown section, and a bad enum are reported in one run, in that order", () => {
     const { io } = captureIo();
@@ -493,6 +543,194 @@ describe("validateSettingsDoc", () => {
     },
   );
 
+  // The class the removal walk passed over: a list of mappings the registry does not key. The marker removes nothing
+  // there, and it rode to GitHub as a field named _remove with nothing said.
+  test.each<[list: string, doc: Record<string, unknown>, site: string]>([
+    [
+      "a ruleset's bypass_actors",
+      ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: true }] }),
+      "rulesets[0].bypass_actors[0]._remove",
+    ],
+    [
+      "a rule's required_status_checks, below a nested keyed entry",
+      ruleset({
+        rules: [
+          {
+            type: "required_status_checks",
+            parameters: {
+              strict_required_status_checks_policy: false,
+              required_status_checks: [{ context: "ci", _remove: true }],
+            },
+          },
+        ],
+      }),
+      "rulesets[0].rules[0].parameters.required_status_checks[0]._remove",
+    ],
+    [
+      "a rule's workflows",
+      ruleset({
+        rules: [
+          {
+            type: "workflows",
+            parameters: { workflows: [{ path: "ci.yml", repository_id: 1, _remove: true }] },
+          },
+        ],
+      }),
+      "rulesets[0].rules[0].parameters.workflows[0]._remove",
+    ],
+    [
+      "a rule's code_scanning_tools",
+      ruleset({
+        rules: [
+          {
+            type: "code_scanning",
+            parameters: {
+              code_scanning_tools: [
+                {
+                  tool: "CodeQL",
+                  alerts_threshold: "errors",
+                  security_alerts_threshold: "high_or_higher",
+                  _remove: true,
+                },
+              ],
+            },
+          },
+        ],
+      }),
+      "rulesets[0].rules[0].parameters.code_scanning_tools[0]._remove",
+    ],
+    [
+      "a pull_request rule's required_reviewers",
+      pullRequestRuleset({
+        required_reviewers: [
+          {
+            file_patterns: ["*.ts"],
+            minimum_approvals: 1,
+            reviewer: { id: 1, type: "Team" },
+            _remove: true,
+          },
+        ],
+      }),
+      "rulesets[0].rules[0].parameters.required_reviewers[0]._remove",
+    ],
+    [
+      "a pull_request rule's dismissal_restriction.allowed_actors",
+      pullRequestRuleset({
+        dismissal_restriction: {
+          enabled: true,
+          allowed_actors: [{ id: 1, type: "Team", _remove: true }],
+        },
+      }),
+      "rulesets[0].rules[0].parameters.dismissal_restriction.allowed_actors[0]._remove",
+    ],
+    [
+      "a mapping section's list",
+      {
+        check_suite_preferences: {
+          auto_trigger_checks: [{ app_id: 1, setting: true, _remove: true }],
+        },
+      },
+      "check_suite_preferences.auto_trigger_checks[0]._remove",
+    ],
+    [
+      "a list under a passthrough key",
+      { repository: { custom_list: [{ a: 1, _remove: true }] } },
+      "repository.custom_list[0]._remove",
+    ],
+    [
+      "a list of a wrapper's entry, spelled through .entries, the marker's value unjudged",
+      {
+        rulesets: {
+          entries: ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: "yes" }] }).rulesets,
+        },
+      },
+      "rulesets.entries[0].bypass_actors[0]._remove",
+    ],
+  ])(
+    "_remove on an entry of %s draws a warning naming its path, and the entry stays as written",
+    (_list, doc, site) => {
+      const { io, annotations } = captureIo();
+      const validated = validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io);
+      expect(annotations).toEqual([`warning: ${unkeyedRemovalNote("s.yml", site)}`]);
+      expect(JSON.stringify(validated._unsafeUnwrap())).toContain('"_remove":');
+    },
+  );
+
+  test("the warning for a marker no list keys says what becomes of it and the fix; a keyed removal earlier in the list is still refused, and each is named by its index as written", () => {
+    const { io, annotations } = captureIo();
+    const doc = {
+      rulesets: [
+        { name: "old", _remove: true },
+        ...ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: true }] }).rulesets,
+      ],
+    };
+    expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io)).toEqual(
+      err({
+        code: "settings-malformed-sections",
+        source: "s.yml",
+        issues: [singleDocumentRemovalIssue("rulesets[0]._remove")],
+      }),
+    );
+    expect(annotations).toEqual([
+      "warning: s.yml: rulesets[1].bypass_actors[0]._remove: _remove belongs on an entry of a keyed list, and this list is not keyed, so it removes nothing and passes through as a field of the entry. Remove the key; the next major version refuses it",
+    ]);
+  });
+
+  // The way back reads a path's steps, so a key that only looks like one must not be read as one: unquoted, the
+  // `[0]` of the key below was renumbered with the rules list the removal shifted.
+  test("a passthrough key spelled like a list step is quoted in the path a warning names, as an issue's is, so a removal in the list it resembles never renumbers it", () => {
+    const { io, annotations } = captureIo();
+    const doc = ruleset({
+      rules: [{ type: "creation", _remove: true }, { type: "deletion" }],
+      "rules[0]": [{ _remove: true }],
+    });
+    expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io)).toEqual(
+      err({
+        code: "settings-malformed-sections",
+        source: "s.yml",
+        issues: [singleDocumentRemovalIssue("rulesets[0].rules[0]._remove")],
+      }),
+    );
+    expect(annotations).toEqual([
+      `warning: ${unkeyedRemovalNote("s.yml", 'rulesets[0]["rules[0]"][0]._remove')}`,
+    ]);
+  });
+
+  // A layer is judged as its standalone view, the keyed removals gone, so the view's index is not the file's; the
+  // wrapper form is the one whose path the way back reads through .entries.
+  test("a layer's view hands over its way back, so a warning names the wrapper's entry by its index in the layer as written", () => {
+    const { io, annotations } = captureIo();
+    const view = standaloneView({
+      rulesets: {
+        entries: [
+          { name: "old", _remove: true },
+          ...ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: true }] }).rulesets,
+        ],
+      },
+    });
+    const options = { asWritten: view.asWritten };
+    expect(
+      validateSettingsDoc(view.doc, "repo.yml", SectionSelection.ALL, io, options).isOk(),
+    ).toBe(true);
+    expect(annotations).toEqual([
+      `warning: ${unkeyedRemovalNote("repo.yml", "rulesets.entries[1].bypass_actors[0]._remove")}`,
+    ]);
+  });
+
+  test("control: a cyclic passthrough value still ends in the boundary's refusal, since the marker walk does not follow a cycle", () => {
+    const loop: Record<string, unknown> = { list: [] };
+    (loop.list as unknown[]).push(loop);
+    expect(
+      validateSettingsDoc({ repository: { loop } }, "s.yml", SectionSelection.ALL, silentIo()),
+    ).toEqual(
+      err({
+        code: "settings-malformed-sections",
+        source: "s.yml",
+        issues: [expect.stringContaining("refers back to one of its own containers")],
+      }),
+    );
+  });
+
   test("a list whose named property shadows a method is refused by the plainness check, never met by the removal walk", () => {
     // The walk for removals runs on the raw document, before the shapes; calling the list's own forEach would throw here.
     expect(
diff --git a/test/flows/layers.test.ts b/test/flows/layers.test.ts
index 0ba59bf80..b63dc4798 100644
--- a/test/flows/layers.test.ts
+++ b/test/flows/layers.test.ts
@@ -2,11 +2,13 @@ import { describe, expect, spyOn, test } from "bun:test";
 import { writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { err, ok } from "neverthrow";
+import { unkeyedRemovalNote } from "../../src/engine/directives.js";
 import type { Layering } from "../../src/engine/layers.js";
 import { foldLayers, readLayerFiles } from "../../src/flows/layers.js";
 import * as settingsRead from "../../src/flows/settings-read.js";
 import { silentIo } from "../../src/io.js";
 import { describeProblem } from "../../src/problem.js";
+import { captureIo } from "../io/capture.js";
 import { withTempDir } from "../temp-dir.js";
 
 describe("readLayerFiles", () => {
@@ -179,6 +181,37 @@ describe("foldLayers", () => {
     },
   );
 
+  // A warning is no issue, so nothing renumbered it: the layer's view index went out, one short per removal before it.
+  test("a layer's warning names the entry by its index in the file as written, the keyed removal before it counted and still dropping its lower entry", () => {
+    const ruleset = (name: string) => ({ name, target: "branch", enforcement: "active" });
+    const marked = { actor_id: 1, actor_type: "Team", bypass_mode: "always", _remove: true };
+    const { io, annotations } = captureIo();
+    const folded = foldLayers(
+      [
+        { name: "fleet.yml", doc: { rulesets: [ruleset("old"), ruleset("r")] } },
+        {
+          name: "repo.yml",
+          doc: {
+            rulesets: [
+              { name: "old", _remove: true },
+              { name: "r", bypass_actors: [marked] },
+            ],
+          },
+        },
+      ],
+      "merged",
+      { layering: "deep" },
+      io,
+    );
+    expect(folded.map((out) => out.notices)).toEqual(
+      ok([{ layer: "repo.yml", path: "rulesets[0]" }]),
+    );
+    expect(annotations).toEqual([
+      `warning: ${unkeyedRemovalNote("repo.yml", "rulesets[1].bypass_actors[0]._remove")}`,
+      `warning: ${unkeyedRemovalNote("merged", "rulesets.entries[0].bypass_actors[0]._remove")}`,
+    ]);
+  });
+
   test("a malformed marker inside a closed entry schema is the fold's refusal, not the layer's own unknown-key problem", () => {
     // Per-layer validation runs before the fold, and an environment secret's schema is closed: were the marker left in
     // the standalone view, the layer would fail on `Unrecognized key: "_remove"` and never hear that it takes only true.

From 1c051db9f51156205ef2866d70ff6c3523ec9dcc Mon Sep 17 00:00:00 2001
From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com>
Date: Wed, 11 Nov 2026 23:20:53 -0500
Subject: [PATCH 2/5] fix(engine): warn of an unread directive only in a
 document that runs

The two warnings printed before the document's issues were known, so a refused document drew them too. For a marker in a closed entry shape (a branch protection's required_status_checks.checks item) the run printed the shape's refusal and, beside it, a warning that the key "passes through as a field" and that "the next major version refuses it": the first untrue of a key just refused, the second already done.

validateSettingsDoc now prints both warnings once the document has no issue. A document that validates has no removal site, so its paths are the document's own and separateRemovals no longer renumbers them itself; a layer's standalone view still hands over its way back. The case that expected a warning beside a refused keyed removal is replaced by two cases pinning that a refused document draws none, and the quoted-key case now runs through a layer's view, the one place a removal may stand beside the marker in a document that validates.

Two lines over the 256-character cap are brought under it: a doc comment, and the pinned warning text in the test.
---
 src/engine/orchestrate.ts       |  19 +++---
 src/engine/separate-removals.ts |   7 +--
 test/engine/orchestrate.test.ts | 102 +++++++++++++++++++++-----------
 3 files changed, 80 insertions(+), 48 deletions(-)

diff --git a/src/engine/orchestrate.ts b/src/engine/orchestrate.ts
index 58e5e78da..a005820d3 100644
--- a/src/engine/orchestrate.ts
+++ b/src/engine/orchestrate.ts
@@ -209,15 +209,6 @@ export function validateSettingsDoc(
   // key, so judged as written it would fail every other required field, and a closed shape would name the marker again.
   const removals = separateRemovals(settings);
   issues.push(...removals.sites.map(singleDocumentRemovalIssue));
-  // COMPAT(v4): a single document whose top-level _layering is outside its values still runs, under a warning; delete the warning and refuse the value as _undeclared's is refused above
-  // COMPAT(v4): a _remove on an entry no list keys still runs and passes through as a field, under a warning; delete the warning and refuse each path with the removal sites above
-  unreadDirectiveNotes(sourceLabel, {
-    layering: (settings as Record<string, unknown>)._layering,
-    unkeyed: removals.unkeyed,
-    asWritten: options.asWritten,
-  }).forEach((note) => {
-    io.annotate("warning", note);
-  });
   const shapes = validateSectionShapes(
     (removals.sites.length === 0 ? settings : removals.rest) as Record<string, unknown>,
     sourceLabel,
@@ -229,6 +220,16 @@ export function validateSettingsDoc(
   if (issues.length > 0) {
     return err({ code: "settings-malformed-sections", source: sourceLabel, issues });
   }
+  // Only a document that runs is warned: in a refused one a closed shape may have named the marker itself.
+  // COMPAT(v4): a single document whose top-level _layering is outside its values still runs, under a warning; delete the warning and refuse the value as _undeclared's is refused above
+  // COMPAT(v4): a _remove on an entry no list keys still runs and passes through as a field, under a warning; delete the warning and refuse each path with the removal sites above
+  unreadDirectiveNotes(sourceLabel, {
+    layering: (settings as Record<string, unknown>)._layering,
+    unkeyed: removals.unkeyed,
+    asWritten: options.asWritten,
+  }).forEach((note) => {
+    io.annotate("warning", note);
+  });
   return shapes.map((parsed) => mintValidatedSettings(parsed, policy ?? options.undeclared));
 }
 
diff --git a/src/engine/separate-removals.ts b/src/engine/separate-removals.ts
index 7a5d77b72..339a5ad24 100644
--- a/src/engine/separate-removals.ts
+++ b/src/engine/separate-removals.ts
@@ -35,7 +35,7 @@ interface Gathered {
   /** Where each kept entry of a shifted list sat in the document as written. */
   readonly sources: SourceIndices;
 
-  /** Each marker on an entry no list keys, by its path as validation spells one over the document minus its removals. */
+  /** Each marker on an entry no list keys, by its path as validation spells one over the kept entries. */
   readonly unkeyed: string[];
 }
 
@@ -195,7 +195,7 @@ export interface SeparatedRemovals {
   /** Each removal's site (`labels[0]._remove`): list sections in LIST_SECTIONS order, each list as written, nested lists under their entry. */
   readonly sites: readonly string[];
 
-  /** Each `_remove` on an entry no list keys, which stays in `rest` with its marker: by its path as validation spells one (`rulesets.entries[0].bypass_actors[0]._remove`), indices as written, the list sections first. */
+  /** Each `_remove` on an entry no list keys, which stays in `rest`: by its path over `rest` as validation spells one (`rulesets.entries[0].bypass_actors[0]._remove`), the list sections first; `asWritten` is its way back. */
   readonly unkeyed: readonly string[];
 
   /** A validation issue over `rest`, its leading path renumbered to the document as written: a removal dropped from the list shifts every entry after it, so `labels[0].color` in `rest` is the reader's `labels[1].color`. */
@@ -235,6 +235,5 @@ export function separateRemovals(doc: unknown): SeparatedRemovals {
     gatherUnkeyed(doc[key], [key], gathered.unkeyed, new Set());
   }
 
-  const asWritten = renumbering(gathered.sources);
-  return { rest, sites, unkeyed: gathered.unkeyed.map(asWritten), asWritten };
+  return { rest, sites, unkeyed: gathered.unkeyed, asWritten: renumbering(gathered.sources) };
 }
diff --git a/test/engine/orchestrate.test.ts b/test/engine/orchestrate.test.ts
index d8680e842..d31111c24 100644
--- a/test/engine/orchestrate.test.ts
+++ b/test/engine/orchestrate.test.ts
@@ -379,7 +379,9 @@ describe("validateSettingsDoc", () => {
       const doc = { _layering: value, labels: [{ name: "bug" }] };
       expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io).isOk()).toBe(true);
       expect(annotations).toEqual([
-        `warning: s.yml: _layering must be one of "replace", "shallow", "deep"; got ${got}. A single document is not folded, so the directive is not read and the run goes on; mode: render refuses this file as a layer. Write one of the values, or remove the key; the next major version refuses it here too`,
+        `warning: s.yml: _layering must be one of "replace", "shallow", "deep"; got ${got}. ` +
+          "A single document is not folded, so the directive is not read and the run goes on; mode: render refuses this file as a layer. " +
+          "Write one of the values, or remove the key; the next major version refuses it here too",
       ]);
     },
   );
@@ -545,12 +547,20 @@ describe("validateSettingsDoc", () => {
 
   // The class the removal walk passed over: a list of mappings the registry does not key. The marker removes nothing
   // there, and it rode to GitHub as a field named _remove with nothing said.
+  test("the warning for a marker no list keys says what becomes of it and the fix, and the validated document keeps the entry as written", () => {
+    const { io, annotations } = captureIo();
+    const doc = ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: true }] });
+    const validated = validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io);
+    expect(annotations).toEqual([
+      "warning: s.yml: rulesets[0].bypass_actors[0]._remove: _remove belongs on an entry of a keyed list, and this list is not keyed, so it removes nothing and passes through as a field of the entry. Remove the key; the next major version refuses it",
+    ]);
+    expect(JSON.stringify(validated._unsafeUnwrap())).toContain(
+      '"bypass_actors":[{"actor_id":1,"actor_type":"Team","bypass_mode":"always","_remove":true}]',
+    );
+  });
+
+  // The rest of the class, one list each: the same warning by its own path.
   test.each<[list: string, doc: Record<string, unknown>, site: string]>([
-    [
-      "a ruleset's bypass_actors",
-      ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: true }] }),
-      "rulesets[0].bypass_actors[0]._remove",
-    ],
     [
       "a rule's required_status_checks, below a nested keyed entry",
       ruleset({
@@ -656,43 +666,65 @@ describe("validateSettingsDoc", () => {
     },
   );
 
-  test("the warning for a marker no list keys says what becomes of it and the fix; a keyed removal earlier in the list is still refused, and each is named by its index as written", () => {
-    const { io, annotations } = captureIo();
-    const doc = {
-      rulesets: [
-        { name: "old", _remove: true },
-        ...ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: true }] }).rulesets,
-      ],
-    };
-    expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io)).toEqual(
-      err({
-        code: "settings-malformed-sections",
-        source: "s.yml",
-        issues: [singleDocumentRemovalIssue("rulesets[0]._remove")],
-      }),
-    );
-    expect(annotations).toEqual([
-      "warning: s.yml: rulesets[1].bypass_actors[0]._remove: _remove belongs on an entry of a keyed list, and this list is not keyed, so it removes nothing and passes through as a field of the entry. Remove the key; the next major version refuses it",
-    ]);
-  });
+  // The warnings speak of a document that runs. Beside a refusal the marker's one said "passes through" of a key a
+  // closed shape had just refused, and promised for the next major what this run already did.
+  test.each<[what: string, doc: Record<string, unknown>, issue: string | RegExp]>([
+    [
+      "a closed entry shape names the marker itself",
+      {
+        branches: [
+          {
+            name: "main",
+            protection: {
+              required_status_checks: { strict: true, checks: [{ context: "ci", _remove: true }] },
+            },
+          },
+        ],
+      },
+      /^branches\[0\]\.protection\.required_status_checks\.checks\[0\]: .* remove "_remove"$/,
+    ],
+    [
+      "a keyed removal stops it, a bad top-level _layering beside it",
+      {
+        _layering: "union",
+        rulesets: [
+          { name: "old", _remove: true },
+          ...ruleset({ bypass_actors: [{ ...TEAM_ACTOR, _remove: true }] }).rulesets,
+        ],
+      },
+      singleDocumentRemovalIssue("rulesets[0]._remove"),
+    ],
+  ])(
+    "a refused document draws no warning for a directive nothing reads: %s",
+    (_what, doc, issue) => {
+      const { io, annotations } = captureIo();
+      expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io)).toEqual(
+        err({
+          code: "settings-malformed-sections",
+          source: "s.yml",
+          issues: [typeof issue === "string" ? issue : expect.stringMatching(issue)],
+        }),
+      );
+      expect(annotations).toEqual([]);
+    },
+  );
 
   // The way back reads a path's steps, so a key that only looks like one must not be read as one: unquoted, the
   // `[0]` of the key below was renumbered with the rules list the removal shifted.
   test("a passthrough key spelled like a list step is quoted in the path a warning names, as an issue's is, so a removal in the list it resembles never renumbers it", () => {
     const { io, annotations } = captureIo();
-    const doc = ruleset({
-      rules: [{ type: "creation", _remove: true }, { type: "deletion" }],
-      "rules[0]": [{ _remove: true }],
-    });
-    expect(validateSettingsDoc(doc, "s.yml", SectionSelection.ALL, io)).toEqual(
-      err({
-        code: "settings-malformed-sections",
-        source: "s.yml",
-        issues: [singleDocumentRemovalIssue("rulesets[0].rules[0]._remove")],
+    const view = standaloneView(
+      ruleset({
+        rules: [{ type: "creation", _remove: true }, { type: "deletion" }],
+        "rules[0]": [{ _remove: true }],
       }),
     );
+    const options = { asWritten: view.asWritten };
+    expect(
+      validateSettingsDoc(view.doc, "repo.yml", SectionSelection.ALL, io, options).isOk(),
+    ).toBe(true);
     expect(annotations).toEqual([
-      `warning: ${unkeyedRemovalNote("s.yml", 'rulesets[0]["rules[0]"][0]._remove')}`,
+      `warning: ${unkeyedRemovalNote("repo.yml", 'rulesets[0]["rules[0]"][0]._remove')}`,
     ]);
   });
 

From ed19ed498c7e1b568a4926fca4f82dd17297a51f Mon Sep 17 00:00:00 2001
From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com>
Date: Wed, 11 Nov 2026 23:23:22 -0500
Subject: [PATCH 3/5] docs(layering): say the unkeyed _remove warning is a
 validated document's

The page said of a _remove on an entry of a list that is not keyed that "the run warns". A document the shapes refuse draws no warning, since a closed entry shape names the marker in its own refusal, so the sentence now says a document that validates draws it.
---
 docs/operate/layering.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/operate/layering.md b/docs/operate/layering.md
index aa3809fff..2ebe0356b 100644
--- a/docs/operate/layering.md
+++ b/docs/operate/layering.md
@@ -76,7 +76,7 @@ Beside these, `_remove: true` on a keyed entry drops the lower entry under that
 
 Any other underscore key at the top level or on a wrapper fails validation with an error naming which directive belongs where (a note belongs in a YAML comment). A misspelled directive can therefore never pass as a private note and quietly merge a layer meant to replace.
 
-Inside a keyed entry, an underscore key is a field like any other: a closed entry shape refuses it as unrecognized, and an open one keeps it, as it keeps every unknown key. A `_remove` on an entry of a list that is not keyed (a ruleset's `bypass_actors`) is such a field: it removes nothing, and the run warns, naming its path.
+Inside a keyed entry, an underscore key is a field like any other: a closed entry shape refuses it as unrecognized, and an open one keeps it, as it keeps every unknown key. A `_remove` on an entry of a list that is not keyed (a ruleset's `bypass_actors`) is such a field: it removes nothing, and a document that validates draws a warning naming its path.
 
 ## The rules
 

From 7f3c81bf697185c1bc98417b2b7f53ce17275291 Mon Sep 17 00:00:00 2001
From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com>
Date: Thu, 12 Nov 2026 00:19:30 -0500
Subject: [PATCH 4/5] fix(flows): print a fold's validation warnings only once
 the fold stands

A layer is validated as its standalone view, which sets aside what the fold alone judges, so its warning for a _remove no list keys went out before the fold refused the layer: a top-level _layering outside its values, a keyed marker that is not true, a wrapper's bad _layering, or another layer the shapes refuse. The line said the key "passes through as a field" of a render that then wrote nothing.

foldLayers now routes every validation of the fold, each layer's and the merged document's, through capturingIo and replays the held lines in their recorded order once the whole fold stands; a refused fold drops them. A fold that stands prints what it printed before, in the same order: the layers' warnings in layer order, then the merged document's.

Three scenarios pin the two warnings in the shipped bundle: a single document whose top-level _layering is outside its values, a single document with a _remove on an entry no list keys, and a layered render where a keyed removal precedes such an entry, so its path counts the removal. The first is raw text: the published schema's enum refuses the value the run still accepts until the next major, and the corpus's schema check holds every settings mapping to that schema.
---
 src/flows/layers.ts                           | 21 +++++-
 .../apply-layering-outside-values-warns.yml   | 27 ++++++++
 .../scenarios/apply-unkeyed-remove-warns.yml  | 32 +++++++++
 .../scenarios/render-unkeyed-remove-warns.yml | 54 +++++++++++++++
 test/flows/layers.test.ts                     | 66 ++++++++++++++++++-
 5 files changed, 195 insertions(+), 5 deletions(-)
 create mode 100644 test/e2e/scenarios/apply-layering-outside-values-warns.yml
 create mode 100644 test/e2e/scenarios/apply-unkeyed-remove-warns.yml
 create mode 100644 test/e2e/scenarios/render-unkeyed-remove-warns.yml

diff --git a/src/flows/layers.ts b/src/flows/layers.ts
index 4112513a7..b7c19e3ba 100644
--- a/src/flows/layers.ts
+++ b/src/flows/layers.ts
@@ -13,6 +13,7 @@ import { type ValidatedSettings, validateSettingsDoc } from "../engine/orchestra
 import { SectionSelection } from "../engine/section-selection.js";
 import type { Io } from "../io.js";
 import type { LayerProblem, ProblemOf, SettingsProblem } from "../problem.js";
+import { capturingIo } from "./redact.js";
 import { readSettingsFile } from "./settings-read.js";
 
 export function readLayerFiles(
@@ -60,6 +61,9 @@ function validateLayer(layer: Layer, io: Io): Result<ValidatedSettings, Settings
  * A layer must be a valid document before it may contribute, so the merge can never complete a broken declaration into
  * a valid one. The fold, not the validated parse, is what is written: the file holds what the layers declared, and
  * validation only judges it.
+ *
+ * What the validations print is held until the fold stands, and dropped when it is refused: a layer's standalone view
+ * validates without what the fold alone judges, so its warning would speak of a run the fold then refuses.
  */
 export function foldLayers(
   layers: readonly Layer[],
@@ -67,11 +71,12 @@ export function foldLayers(
   options: FoldOptions,
   io: Io,
 ): Result<FoldedLayers, SettingsProblem | LayerProblem> {
-  return Result.combine(layers.map((layer) => validateLayer(layer, io)))
+  const held = capturingIo(io);
+  return Result.combine(layers.map((layer) => validateLayer(layer, held.io)))
     .andThen(() => mergeLayers(layers, options))
     .andThen((merged) =>
       // The fold resolved every policy already, so the run input changes nothing here; passed so the two paths read alike.
-      validateSettingsDoc(merged.settings, sourceLabel, EVERY_SECTION, io, {
+      validateSettingsDoc(merged.settings, sourceLabel, EVERY_SECTION, held.io, {
         undeclared: options.undeclared,
       }).map((settings) => ({
         settings,
@@ -79,5 +84,15 @@ export function foldLayers(
         // Validation just proved the fold a plain mapping of section keys: the directives were consumed, and any other key refused.
         yaml: renderCanonicalYaml(merged.settings as Record<string, unknown>),
       })),
-    );
+    )
+    .map((folded) => {
+      for (const { level, line } of held.drain()) {
+        if (level === undefined) {
+          io.log(line);
+        } else {
+          io.annotate(level, line);
+        }
+      }
+      return folded;
+    });
 }
diff --git a/test/e2e/scenarios/apply-layering-outside-values-warns.yml b/test/e2e/scenarios/apply-layering-outside-values-warns.yml
new file mode 100644
index 000000000..4a8b07511
--- /dev/null
+++ b/test/e2e/scenarios/apply-layering-outside-values-warns.yml
@@ -0,0 +1,27 @@
+# A single document's top-level `_layering` outside replace|shallow|deep is read
+# by nothing, since no fold runs: the apply goes on as it always did, now under
+# one warning naming the values and the fix. mode: render refuses the same file
+# as a layer. Raw text, because the published schema refuses the value and the
+# corpus's schema check holds every `settings` mapping to it.
+name: apply-layering-outside-values-warns
+settings_raw: |
+  _layering: banana
+  labels:
+    - name: bug
+      color: d73a4a
+expect:
+  exit_code: 0
+  result: applied
+  outcomes:
+    labels: applied
+  mutations:
+    - "POST /repos/{repo}/labels"
+  stdout_contains:
+    - "::warning::"
+    - >-
+      settings.yml: _layering must be one of "replace", "shallow", "deep"; got a string that is none of them.
+      A single document is not folded, so the directive is not read and the run goes on;
+      mode: render refuses this file as a layer.
+      Write one of the values, or remove the key; the next major version refuses it here too
+    - 'labels: created label "bug"'
+  fixpoint: converges
diff --git a/test/e2e/scenarios/apply-unkeyed-remove-warns.yml b/test/e2e/scenarios/apply-unkeyed-remove-warns.yml
new file mode 100644
index 000000000..5368e8118
--- /dev/null
+++ b/test/e2e/scenarios/apply-unkeyed-remove-warns.yml
@@ -0,0 +1,32 @@
+# `_remove` on an entry of a list the registry does not key (a ruleset's
+# bypass_actors) removes nothing: the entry is applied as written, the marker
+# one of its fields as before, and the run now warns, naming the entry's path.
+name: apply-unkeyed-remove-warns
+settings:
+  rulesets:
+    - name: r
+      target: branch
+      enforcement: active
+      bypass_actors:
+        - actor_id: 1
+          actor_type: Team
+          bypass_mode: always
+          _remove: true
+token_permissions:
+  administration: write
+expect:
+  exit_code: 0
+  result: applied
+  outcomes:
+    rulesets: applied
+  mutations:
+    - "POST /repos/{repo}/rulesets"
+  stdout_contains:
+    - "::warning::"
+    - >-
+      settings.yml: rulesets[0].bypass_actors[0]._remove:
+      _remove belongs on an entry of a keyed list, and this list is not keyed,
+      so it removes nothing and passes through as a field of the entry.
+      Remove the key; the next major version refuses it
+    - 'rulesets: created ruleset "r"'
+  fixpoint: converges
diff --git a/test/e2e/scenarios/render-unkeyed-remove-warns.yml b/test/e2e/scenarios/render-unkeyed-remove-warns.yml
new file mode 100644
index 000000000..84eeffe8d
--- /dev/null
+++ b/test/e2e/scenarios/render-unkeyed-remove-warns.yml
@@ -0,0 +1,54 @@
+# A layer's warning names the entry by its index in the file as written: the
+# keyed removal before it is counted (rulesets[1], where the layer's standalone
+# view holds it at 0) and still drops the lower entry. The marker rides into
+# the rendered document as before, which the fold names once more by that
+# document's own path. The warnings print before the removal's notice.
+name: render-unkeyed-remove-warns
+inputs:
+  mode: render
+settings_layers:
+  - rulesets:
+      - name: old
+        target: branch
+        enforcement: active
+      - name: r
+        target: branch
+        enforcement: active
+settings:
+  rulesets:
+    - name: old
+      _remove: true
+    - name: r
+      bypass_actors:
+        - actor_id: 1
+          actor_type: Team
+          bypass_mode: always
+          _remove: true
+expect:
+  exit_code: 0
+  result: rendered
+  zero_requests: true
+  stdout_contains:
+    - >-
+      settings.yml: rulesets[1].bypass_actors[0]._remove:
+      _remove belongs on an entry of a keyed list, and this list is not keyed,
+      so it removes nothing and passes through as a field of the entry.
+      Remove the key; the next major version refuses it
+    - >-
+      ::warning::the rendered settings document: rulesets.entries[0].bypass_actors[0]._remove:
+      _remove belongs on an entry of a keyed list, and this list is not keyed,
+      so it removes nothing and passes through as a field of the entry.
+      Remove the key; the next major version refuses it
+    - "settings.yml: rulesets[0] carries _remove: true and dropped the entry a lower layer declared under its key"
+  rendered:
+    rulesets:
+      _undeclared: keep
+      entries:
+        - name: r
+          target: branch
+          enforcement: active
+          bypass_actors:
+            - actor_id: 1
+              actor_type: Team
+              bypass_mode: always
+              _remove: true
diff --git a/test/flows/layers.test.ts b/test/flows/layers.test.ts
index b63dc4798..b8624726c 100644
--- a/test/flows/layers.test.ts
+++ b/test/flows/layers.test.ts
@@ -56,6 +56,9 @@ describe("readLayerFiles", () => {
 
 describe("foldLayers", () => {
   const fleet = { name: "fleet.yml", doc: { labels: [{ name: "bug", description: "fleet" }] } };
+
+  /** A repository section whose passthrough list carries a marker no list keys: valid, and warned of. */
+  const MARKED_REPOSITORY = { custom_list: [{ _remove: true }] };
   const fold = (doc: Record<string, unknown>, layering: Layering) =>
     foldLayers([fleet, { name: "repo.yml", doc }], "merged", { layering }, silentIo());
 
@@ -182,13 +185,19 @@ describe("foldLayers", () => {
   );
 
   // A warning is no issue, so nothing renumbered it: the layer's view index went out, one short per removal before it.
-  test("a layer's warning names the entry by its index in the file as written, the keyed removal before it counted and still dropping its lower entry", () => {
+  test("a layer's warning names the entry by its index in the file as written, the keyed removal before it counted and still dropping its lower entry; the warnings print in layer order, the fold's own last", () => {
     const ruleset = (name: string) => ({ name, target: "branch", enforcement: "active" });
     const marked = { actor_id: 1, actor_type: "Team", bypass_mode: "always", _remove: true };
     const { io, annotations } = captureIo();
     const folded = foldLayers(
       [
-        { name: "fleet.yml", doc: { rulesets: [ruleset("old"), ruleset("r")] } },
+        {
+          name: "fleet.yml",
+          doc: {
+            rulesets: [ruleset("old"), ruleset("r")],
+            repository: { custom_list: [{ _remove: true }] },
+          },
+        },
         {
           name: "repo.yml",
           doc: {
@@ -207,11 +216,64 @@ describe("foldLayers", () => {
       ok([{ layer: "repo.yml", path: "rulesets[0]" }]),
     );
     expect(annotations).toEqual([
+      `warning: ${unkeyedRemovalNote("fleet.yml", "repository.custom_list[0]._remove")}`,
       `warning: ${unkeyedRemovalNote("repo.yml", "rulesets[1].bypass_actors[0]._remove")}`,
       `warning: ${unkeyedRemovalNote("merged", "rulesets.entries[0].bypass_actors[0]._remove")}`,
+      `warning: ${unkeyedRemovalNote("merged", "repository.custom_list[0]._remove")}`,
     ]);
   });
 
+  // A layer's standalone view validates without what the fold alone judges, so its warning went out ahead of the
+  // fold's refusal: "passes through as a field" printed for a render that wrote nothing.
+  test.each<[what: string, above: { name: string; doc: Record<string, unknown> }[], code: string]>([
+    [
+      "its own top-level _layering is outside the values",
+      [{ name: "repo.yml", doc: { _layering: "banana", repository: MARKED_REPOSITORY } }],
+      "layer-bad-directive",
+    ],
+    [
+      "a keyed marker beside it is not true",
+      [
+        {
+          name: "repo.yml",
+          doc: { labels: [{ name: "bug", _remove: false }], repository: MARKED_REPOSITORY },
+        },
+      ],
+      "layer-remove-not-true",
+    ],
+    [
+      "a wrapper's _layering beside it is outside the values",
+      [
+        {
+          name: "repo.yml",
+          doc: {
+            labels: { _layering: "union", entries: [{ name: "docs" }] },
+            repository: MARKED_REPOSITORY,
+          },
+        },
+      ],
+      "layer-bad-directive",
+    ],
+    [
+      "another layer is one the shapes refuse",
+      [
+        { name: "team.yml", doc: { repository: MARKED_REPOSITORY } },
+        { name: "repo.yml", doc: { labels: "oops" } },
+      ],
+      "settings-malformed-sections",
+    ],
+  ])("a refused fold prints no layer's warning: %s", (_what, above, code) => {
+    const { io, annotations } = captureIo();
+    const folded = foldLayers([fleet, ...above], "merged", { layering: "deep" }, io);
+    expect<string | null>(
+      folded.match(
+        () => null,
+        (problem) => problem.code,
+      ),
+    ).toBe(code);
+    expect(annotations).toEqual([]);
+  });
+
   test("a malformed marker inside a closed entry schema is the fold's refusal, not the layer's own unknown-key problem", () => {
     // Per-layer validation runs before the fold, and an environment secret's schema is closed: were the marker left in
     // the standalone view, the layer would fail on `Unrecognized key: "_remove"` and never hear that it takes only true.

From 603e71dcdc77b02b3a67c6145483a5e881dcd4f1 Mon Sep 17 00:00:00 2001
From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com>
Date: Thu, 12 Nov 2026 00:24:24 -0500
Subject: [PATCH 5/5] fix(engine): walk a shared marker-free subtree once when
 gathering unkeyed _remove

The walk for a _remove on an entry no list keys visited a shared object once per path. A mapping aliased under two keys, 24 levels deep, is 25 mappings and 33,554,431 paths, so a document holding no marker at all took seconds to say so. The YAML reader caps aliases, but validateSettings is a library call that takes a parsed object.

gatherUnkeyed now returns whether a value is known to hold no marker and remembers each such mapping or list, to skip it when another path meets it again, as the neighbouring walk in validate.ts does. A subtree that holds a marker is still walked per path, so every path keeps its own warning. A container on a cycle is never remembered: the descent ends where the cycle closes, so what lies beyond the cut is not known, and a document whose cycle the shapes drop (an own __proto__ key) still validates.
---
 src/engine/separate-removals.ts       | 74 +++++++++++++++++++--------
 test/engine/separate-removals.test.ts | 50 ++++++++++++++++++
 2 files changed, 104 insertions(+), 20 deletions(-)
 create mode 100644 test/engine/separate-removals.test.ts

diff --git a/src/engine/separate-removals.ts b/src/engine/separate-removals.ts
index 339a5ad24..80f848958 100644
--- a/src/engine/separate-removals.ts
+++ b/src/engine/separate-removals.ts
@@ -37,6 +37,9 @@ interface Gathered {
 
   /** Each marker on an entry no list keys, by its path as validation spells one over the kept entries. */
   readonly unkeyed: string[];
+
+  /** Each mapping or list walked to its end with no such marker found: skipped when another path meets it again. */
+  readonly clean: WeakSet<object>;
 }
 
 /** A keyed list's removals set apart from it, its entries' nested lists likewise, in the form the layer wrote them. */
@@ -48,32 +51,63 @@ interface Partition {
   readonly sites: string[];
 }
 
+/** A list's or a mapping's children, each with the step that reaches it; a list is read by index, since the walk runs before the plainness check that refuses one whose named property shadows a method. */
+function childrenOf(
+  container: readonly unknown[] | Readonly<Record<string, unknown>>,
+): [step: string | number, child: unknown][] {
+  if (!Array.isArray(container)) {
+    return Object.entries(container);
+  }
+  const children: [number, unknown][] = [];
+  for (let index = 0; index < container.length; index += 1) {
+    children.push([index, container[index]]);
+  }
+  return children;
+}
+
 /**
- * The markers below a value no keyed list reaches: a `_remove` on a list's entry there removes nothing (the fold
- * replaces such a list whole), so the entry stays and each marker is gathered by its path. `above` holds the
- * containers on the way down, so a YAML alias cycle ends the descent.
+ * Whether a value no keyed list reaches is known to hold no marker, each one below it gathered by its path: a
+ * `_remove` on a list's entry there removes nothing (the fold replaces such a list whole), so the entry stays.
+ *
+ *   holds no marker  -> remembered, so walked once however many paths a YAML alias gives it, as validate.ts walks one
+ *   holds a marker   -> walked per path, so each path keeps its own line
+ *   closes a cycle   -> the descent ends there (`above`), and no container on the way back up is remembered: what
+ *                       the cut hid is not known
  */
-function gatherUnkeyed(value: unknown, path: Steps, unkeyed: string[], above: Set<object>): void {
-  if ((!Array.isArray(value) && !isPlainObject(value)) || above.has(value)) {
-    return;
+function gatherUnkeyed(
+  value: unknown,
+  path: Steps,
+  gathered: Gathered,
+  above: Set<object>,
+): boolean {
+  if (!Array.isArray(value) && !isPlainObject(value)) {
+    return true;
+  }
+  if (gathered.clean.has(value)) {
+    return true;
+  }
+  if (above.has(value)) {
+    return false;
   }
 
   above.add(value);
-  if (Array.isArray(value)) {
-    // Indexed, as in partitionRemovals: the walk runs before the plainness check.
-    for (let index = 0; index < value.length; index += 1) {
-      const item: unknown = value[index];
-      if (isPlainObject(item) && carriesRemoval(item)) {
-        unkeyed.push(z.core.toDotPath([...path, index, REMOVE_KEY]));
-      }
-      gatherUnkeyed(item, [...path, index], unkeyed, above);
+  const entries = Array.isArray(value);
+  let clean = true;
+  for (const [step, child] of childrenOf(value)) {
+    if (entries && isPlainObject(child) && carriesRemoval(child)) {
+      gathered.unkeyed.push(z.core.toDotPath([...path, step, REMOVE_KEY]));
+      clean = false;
     }
-  } else {
-    for (const [field, child] of Object.entries(value)) {
-      gatherUnkeyed(child, [...path, field], unkeyed, above);
+    if (!gatherUnkeyed(child, [...path, step], gathered, above)) {
+      clean = false;
     }
   }
   above.delete(value);
+
+  if (clean) {
+    gathered.clean.add(value);
+  }
+  return clean;
 }
 
 /** A kept entry with each declared nested list partitioned in turn, and the sites of the removals below it; its other fields are walked for the markers no list keys. */
@@ -107,7 +141,7 @@ function partitionNestedLists(
 
   for (const [field, value] of Object.entries(entry)) {
     if (!partitioned.has(field)) {
-      gatherUnkeyed(value, [...viewSite, field], gathered.unkeyed, new Set());
+      gatherUnkeyed(value, [...viewSite, field], gathered, new Set());
     }
   }
   return { entry: out, sites };
@@ -214,7 +248,7 @@ export function separateRemovals(doc: unknown): SeparatedRemovals {
   }
   const rest: Record<string, unknown> = { ...doc };
   const sites: string[] = [];
-  const gathered: Gathered = { sources: new Map(), unkeyed: [] };
+  const gathered: Gathered = { sources: new Map(), unkeyed: [], clean: new WeakSet() };
   for (const key of LIST_SECTIONS) {
     const form = nestedForm(doc[key]);
     if (form === null) {
@@ -232,7 +266,7 @@ export function separateRemovals(doc: unknown): SeparatedRemovals {
   }
 
   for (const key of MAPPING_SECTIONS) {
-    gatherUnkeyed(doc[key], [key], gathered.unkeyed, new Set());
+    gatherUnkeyed(doc[key], [key], gathered, new Set());
   }
 
   return { rest, sites, unkeyed: gathered.unkeyed, asWritten: renumbering(gathered.sources) };
diff --git a/test/engine/separate-removals.test.ts b/test/engine/separate-removals.test.ts
new file mode 100644
index 000000000..390f9acc5
--- /dev/null
+++ b/test/engine/separate-removals.test.ts
@@ -0,0 +1,50 @@
+import { describe, expect, test } from "bun:test";
+import { separateRemovals } from "../../src/engine/separate-removals.js";
+
+/** A plain mapping that counts each time a walk lists its keys; a Proxy over one still reads as plain YAML data. */
+function counted<T extends object>(target: T, visits: { count: number }): T {
+  return new Proxy(target, {
+    ownKeys: (inner) => {
+      visits.count += 1;
+      return Reflect.ownKeys(inner);
+    },
+  });
+}
+
+describe("separateRemovals: the markers no list keys", () => {
+  // One YAML anchor aliased under two keys, 24 levels deep, is 25 mappings and 2^25 - 1 paths: walked per path,
+  // a library caller's document that holds no marker at all took seconds to say so.
+  test("a mapping shared by many paths is walked once when it holds no marker", () => {
+    const visits = { count: 0 };
+    let shared: object = counted({}, visits);
+    for (let depth = 0; depth < 24; depth += 1) {
+      shared = counted({ left: shared, right: shared }, visits);
+    }
+    expect(separateRemovals({ repository: { custom: shared } }).unkeyed).toEqual([]);
+    expect(visits.count).toBe(25);
+  });
+
+  // A walk a cycle cut short does not know what the cut hid. Remembered as marker-free all the same, the mapping
+  // was skipped on its second path, and a document whose cycle the shapes drop (an own `__proto__` key) lost a line.
+  test("a mapping whose walk a cycle cut short is not remembered as marker-free, so a second path through it still finds the marker", () => {
+    const first: Record<string, unknown> = { custom_list: [{ _remove: true }] };
+    const second = { back: first };
+    first.next = second;
+    expect(separateRemovals({ repository: first, actions: second }).unkeyed).toEqual([
+      "repository.custom_list[0]._remove",
+      "actions.back.custom_list[0]._remove",
+    ]);
+  });
+
+  test("control: a shared list that holds a marker is still walked per path, so every path keeps its own line", () => {
+    const marked = [{ _remove: true }];
+    const quiet = { nested: [{ a: 1 }] };
+    const doc = {
+      repository: { first: marked, calm: quiet, second: { again: marked, calm: quiet } },
+    };
+    expect(separateRemovals(doc).unkeyed).toEqual([
+      "repository.first[0]._remove",
+      "repository.second.again[0]._remove",
+    ]);
+  });
+});