fix(engine): name a notice's section once, and report what a refused entry left unattempted

src/engine/execute.ts

@@ -1,7 +1,7 @@
 /**
  * The ONE place a section's operations touch the API. Operations go through the request helpers so error classification
  * (a denial vs a hard failure, the hints) matches the reads'; a failure comes back as the value the section loop
- * classifies, beside what landed before it.
+ * classifies, beside what landed before it and the operations it left unattempted.
  */
 
 import { err, ok, type Result } from "neverthrow";
@@ -10,7 +10,7 @@ import type { GitHubClient } from "../github/api.js";
 import { type EndpointDecl, endpointMethod } from "../sections/contract/endpoints.js";
 import { type SectionFailure, sectionFailure, thrown } from "../sections/contract/errors.js";
 import type { SectionContext, SectionMeta } from "../sections/contract/module.js";
-import type { ExecTools, SectionPlan } from "../sections/contract/plan.js";
+import { driftOf, type ExecTools, type SectionPlan } from "../sections/contract/plan.js";
 import {
   callDeclared,
   callGraphql,
@@ -25,9 +25,14 @@ interface PlanExecutionBase {
   readonly landed: number;
 }
 
+/** A failure ends the plan, since a later operation may depend on an earlier one; `unattempted` names each operation after it, in plan order. */
 type PlanExecution =
   | (PlanExecutionBase & { readonly status: "applied" })
-  | (PlanExecutionBase & { readonly status: "failed"; readonly failure: SectionFailure });
+  | (PlanExecutionBase & {
+      readonly status: "failed";
+      readonly failure: SectionFailure;
+      readonly unattempted: readonly string[];
+    });
 
 type PlannedOp = SectionPlan["ops"][number];
 
@@ -236,6 +241,19 @@ function changeLines(
   return ok(typeof lines === "string" ? [lines] : lines);
 }
 
+/**
+ * An operation that never ran, in its section's own words: its description, else the drift it would have resolved,
+ * else (a write that recurs without drift and describes nothing) its role.
+ */
+function nameOf(op: PlannedOp): string {
+  if (op.describe !== undefined) {
+    return op.describe;
+  }
+
+  const drift = driftOf(op);
+  return drift.length > 0 ? drift.join("; ") : `the "${op.role}" write`;
+}
+
 export async function executePlan(
   plan: SectionPlan,
   section: SectionMeta,
@@ -250,19 +268,20 @@ export async function executePlan(
   const changes: string[] = [];
   const notes: string[] = [];
   let landed = 0;
-  const failed = (failure: SectionFailure): PlanExecution => ({
+  const failed = (failure: SectionFailure, at: number): PlanExecution => ({
     status: "failed",
     changes,
     notes,
     landed,
     failure,
+    unattempted: plan.ops.slice(at + 1).map(nameOf),
   });
-  for (const op of plan.ops) {
+  for (const [at, op] of plan.ops.entries()) {
     const { exec, resolved } = secretRecorder(tools);
     try {
       const requested = await request(run, op, exec, resolved);
       if (requested.isErr()) {
-        return failed(requested.error);
+        return failed(requested.error, at);
       }
 
       if (requested.value.kind === "tolerated") {
@@ -273,13 +292,13 @@ export async function executePlan(
       landed++;
       const rendered = changeLines(section, op, requested.value.response);
       if (rendered.isErr()) {
-        return failed(rendered.error);
+        return failed(rendered.error, at);
       }
 
       changes.push(...rendered.value);
     } catch (error) {
       // The client's own throw on an unmarked request, or a BUG invariant: reported beside what landed.
-      return failed(thrown(error));
+      return failed(thrown(error), at);
     }
   }
 

src/engine/orchestrate.ts

@@ -121,6 +121,7 @@ interface Produced {
   readonly notes: readonly string[];
   readonly changes: readonly string[];
   readonly landed: number;
+  readonly unattempted: readonly string[];
 }
 
 /** A section's plan and execution as a value, beside what it produced on the way. */
@@ -383,7 +384,7 @@ async function runSection(
     );
   }
 
-  let produced: Produced = { notes: [], changes: [], landed: 0 };
+  let produced: Produced = { notes: [], changes: [], landed: 0, unattempted: [] };
   try {
     const planned = await section.plan(planContext(section, run.api, run.repo), desired);
     if (planned.isErr()) {
@@ -400,7 +401,12 @@ async function runSection(
 
     const execution = await executePlan(plan, section, run.api, run.repo, run.tools);
     const notes = [...plan.notes, ...plan.drift, ...execution.notes];
-    produced = { notes, changes: execution.changes, landed: execution.landed };
+    produced = {
+      notes,
+      changes: execution.changes,
+      landed: execution.landed,
+      unattempted: execution.status === "failed" ? execution.unattempted : [],
+    };
     if (execution.status === "failed") {
       return { step: err(execution.failure), produced };
     }
@@ -411,6 +417,34 @@ async function runSection(
   }
 }
 
+/** A section's line under its key, prefixed once: a section's own lines already lead with the key (`teams: ...`, `repository.enable_git_lfs: ...`, `environments[prod]: ...`). */
+export function underKey(key: SectionKey, line: string): string {
+  const own = line.startsWith(key) && /^[:.[]/.test(line.slice(key.length));
+  return own ? line : `${key}: ${line}`;
+}
+
+/**
+ * What the stop at a failed operation cost, said after the failure: the count as a warning, each operation the plan
+ * still held as a log line, and those lines as the rows they add. The stop itself stays, since a later operation can
+ * depend on the one that failed.
+ */
+function reportUnattempted(io: Io, key: SectionKey, unattempted: readonly string[]): string[] {
+  const count = unattempted.length;
+  if (count === 0) {
+    return [];
+  }
+
+  io.annotate(
+    "warning",
+    `${key}: ${countNoun(count, "operation", "operations")} after this failure ${agree(count, "was", "were")} not attempted; re-run once it is fixed`,
+  );
+  const rows = unattempted.map((name) => `not attempted - ${name}`);
+  for (const row of rows) {
+    io.log(`${key}: ${row}`);
+  }
+  return rows;
+}
+
 /** Why a denial fails the run whatever the policy: a request landed, or the section is required. */
 function denialWhy(landed: number, required: boolean): string {
   if (landed > 0) {
@@ -422,12 +456,15 @@ function denialWhy(landed: number, required: boolean): string {
   return "";
 }
 
-/** A denied section: skipped under warn when nothing landed and the section is not required, else failed, the line saying why the policy did not spare it. */
+/**
+ * A denied section: skipped under warn when nothing landed and the section is not required, else failed, the line
+ * saying why the policy did not spare it. A skip covers every operation of the section, so it lists none unattempted.
+ */
 function deniedOutcome(
   run: SectionRun,
   key: SectionKey,
   failure: Denied,
-  landed: number,
+  { landed, unattempted }: Produced,
   before: readonly string[],
 ): SectionOutcome {
   const required = run.opts.sections.required.has(key);
@@ -444,39 +481,44 @@ function deniedOutcome(
 
   const applied = landed > 0 ? "partially applied" : "not applied";
   run.io.annotate("error", `${key}: ${applied}${denialWhy(landed, required)} - ${failure.detail}`);
-  return { key, status: "failed", detail: [...before, failure.detail], httpStatus: failure.status };
+  const after = reportUnattempted(run.io, key, unattempted);
+  return {
+    key,
+    status: "failed",
+    detail: [...before, failure.detail, ...after],
+    httpStatus: failure.status,
+  };
 }
 
-/** A section that failed: what it produced first (its notes and change lines), then the failure as its row. */
+/** A section that failed: what it produced first (its notes and change lines), then the failure as its row, then what the failure left unattempted. */
 function failedOutcome(
   run: SectionRun,
   key: SectionKey,
   failure: SectionFailure,
   produced: Produced,
 ): SectionOutcome {
   for (const note of produced.notes) {
-    run.io.annotate("notice", `${key}: ${note}`);
+    run.io.annotate("notice", underKey(key, note));
   }
   for (const line of produced.changes) {
-    run.io.log(`${key}: ${line}`);
+    run.io.log(underKey(key, line));
   }
 
   const before = [...produced.notes, ...produced.changes];
   if (failure.kind === "permission-denied") {
-    return deniedOutcome(run, key, failure, produced.landed, before);
+    return deniedOutcome(run, key, failure, produced, before);
   }
 
-  // failureFor() messages already carry section, cause, and fix; anything else gets the section prefixed. A landed
-  // request is a real mutation with or without its line, so say so.
-  const prefixed = failure.message.startsWith(`${key}:`)
-    ? failure.message
-    : `${key}: ${failure.message}`;
+  // failureFor() messages already carry section, cause, and fix, and a refusal leads with its entry; anything else gets
+  // the section prefixed. A landed request is a real mutation with or without its line, so say so.
+  const prefixed = underKey(key, failure.message);
   const annotated =
     produced.landed > 0
       ? `${prefixed} (${countNoun(produced.landed, "request", "requests")} landed before this failure, so the repository is partially applied)`
       : prefixed;
   run.io.annotate("error", annotated);
-  return { key, status: "failed", detail: [...before, annotated] };
+  const after = reportUnattempted(run.io, key, produced.unattempted);
+  return { key, status: "failed", detail: [...before, annotated, ...after] };
 }
 
 /** A section that changed nothing but left notes (a tolerated 409, a personal-account skip) is NOT "already in the desired state"; the notes are shown instead of claiming no changes were needed. */
@@ -493,7 +535,7 @@ function appliedDetail(changes: string[], notes: string[]): string[] {
 /** A section that ran: its notes, then its drift lines (check) or change lines (apply) as its row. */
 function ranOutcome(io: Io, key: SectionKey, outcome: Outcome): SectionOutcome {
   for (const note of outcome.notes) {
-    io.annotate("notice", `${key}: ${note}`);
+    io.annotate("notice", underKey(key, note));
   }
 
   if (outcome.check) {
@@ -507,7 +549,7 @@ function ranOutcome(io: Io, key: SectionKey, outcome: Outcome): SectionOutcome {
   }
 
   for (const line of outcome.changes) {
-    io.log(`${key}: ${line}`);
+    io.log(underKey(key, line));
   }
   return { key, status: "applied", detail: appliedDetail(outcome.changes, outcome.notes) };
 }

src/engine/secret-refs.ts

@@ -30,8 +30,8 @@ export const RESERVED_REF_PREFIXES = ["INPUT_", "GITHUB_", "ACTIONS_", "RUNNER_"
 
 /**
  * Reads no environment, so check mode and preflight run it without touching secrets; `label` names the owning entry
- * (a secret name, a webhook url), never a value. Errors never echo a non-reference value either: a rejected literal,
- * or the text around an embedded `$NAME`, may already be a secret.
+ * (a secret name, a webhook url), never a value. Errors never echo any part of a non-reference value either: a
+ * rejected literal may already be a secret, and so may what reads as an embedded `$NAME` (the tail of `Pa$$W0RD`).
  */
 export function validateSecretRef(
   value: string,
@@ -53,10 +53,9 @@ export function validateSecretRef(
     }
     return ok({ name });
   }
-  const embedded = value.match(EMBEDDED_REFERENCE_RE)?.[0];
-  if (embedded !== undefined) {
+  if (EMBEDDED_REFERENCE_RE.test(value)) {
     return err(
-      `${label} embeds ${embedded} without being a whole-value reference; it would otherwise ship verbatim as the secret. Make the entire value a single $NAME reference`,
+      `${label} embeds a $NAME reference without being a whole-value reference; it would otherwise ship verbatim as the secret. Make the entire value a single $NAME reference`,
     );
   }
   return err(

src/engine/snapshot.ts

@@ -25,7 +25,7 @@ import { type OnMissingPermission, snapshotContext } from "../sections/contract/
 import { SECTIONS } from "../sections/registry.js";
 import type { MustBeNever } from "../types.js";
 import { canonicalDocument, compareByCodePoint, renderCanonicalYaml } from "./canonical.js";
-import { type ValidatedSettings, validateSettingsDoc } from "./orchestrate.js";
+import { underKey, type ValidatedSettings, validateSettingsDoc } from "./orchestrate.js";
 import type { RunOutcome } from "./outcome.js";
 import { SectionSelection } from "./section-selection.js";
 
@@ -77,12 +77,6 @@ type SectionRead =
 /** What a section without live state says for itself in the outcome and the file header. */
 const NOTHING_TO_DECLARE = "nothing exists on the repository, so the section is omitted";
 
-/** A line under its section's key, prefixed once: a section's own notes already lead with the key. */
-function underKey(key: SectionKey, line: string): string {
-  const own = line.startsWith(key) && /^[:.[]/.test(line.slice(key.length));
-  return own ? line : `${key}: ${line}`;
-}
-
 /**
  * The client as one section's snapshot sees it, reporting whether a GET on `read`'s route
  * answered 404 (the observation the concealed-absence note is keyed on). Null passes through.
@@ -135,9 +129,7 @@ function failedSnapshot(
     return { key, status: "failed", detail: [failure.detail] };
   }
 
-  const prefixed = failure.message.startsWith(`${key}:`)
-    ? failure.message
-    : `${key}: ${failure.message}`;
+  const prefixed = underKey(key, failure.message);
   io.annotate("error", prefixed);
   return { key, status: "failed", detail: [prefixed] };
 }