refactor(sections)!: environments plan through the secrets and variables engines; the sealing key is read at apply

docs/upgrading/v2-to-v3.md

@@ -4,7 +4,7 @@ order: 20
 
 # Upgrading from v2 to v3
 
-Eighteen breaks (the ninth is for library consumers). One is silent (the fallback), so run `mode: check` before the first v3 apply. The changelog entry for 3.0.0 will carry the release-please footers in the [CHANGELOG](https://github.com/Vivswan/github-settings-as-code/blob/main/CHANGELOG.md).
+Twenty breaks (the ninth is for library consumers). One is silent (the fallback), so run `mode: check` before the first v3 apply. The changelog entry for 3.0.0 will carry the release-please footers in the [CHANGELOG](https://github.com/Vivswan/github-settings-as-code/blob/main/CHANGELOG.md).
 
 | Break | v2 | v3 | What the old form does now |
 |---|---|---|---|
@@ -26,6 +26,8 @@ Eighteen breaks (the ninth is for library consumers). One is silent (the fallbac
 | Underscore keys are directives, never notes | An unknown `_note: ...` at the top level was dropped silently | Only `_layering` and `_undeclared` exist; any other underscore key fails validation, on a wrapper and at the top level alike | Validation fails before any section runs, naming the two directives; [section 16](#16-underscore-keys-are-directives-never-notes) |
 | `teams` takes the `_undeclared` knob | A plain array; an undeclared team was never listed or touched | `teams: {_undeclared: keep, entries: [...]}` accepted, default `keep`; `delete` revokes undeclared direct grants | No error. Every run now lists the repository's teams and notes each undeclared one; snapshots write the wrapper form; [section 17](#17-teams-takes-the-_undeclared-knob) |
 | `GSAC_RETRY_BASE_MS` | `RETRY_BASE_MS`, undocumented | `GSAC_RETRY_BASE_MS`, in the inputs reference | No error: an unknown environment variable is ignored, so a harness setting the old name waits real seconds; [section 18](#18-gsac_retry_base_ms) |
+| The sealing key is read at apply time | Check mode read `GET .../secrets/public-key` and failed on a malformed key | The first sealed PUT reads it at apply | Check mode issues one request fewer per secret family; a malformed key fails at apply; [section 19](#19-the-sealing-key-is-read-at-apply-time) |
+| Environment secrets and variables plan through the shared engines | Their own wording | The engines' wording | Only a grep over the output notices; [section 20](#20-environment-secrets-and-variables-plan-through-the-shared-engines) |
 
 ## 1. The defaults-file fallback
 
@@ -277,6 +279,27 @@ The plain array form keeps working, and the [undeclared policy](../reference/und
 
 The one environment variable of the tool's own, the retry-timing test knob, is `GSAC_RETRY_BASE_MS`; v2 read it as `RETRY_BASE_MS`, undocumented. An environment variable has no channel for a loud error, so the old name is simply ignored: a harness that set `RETRY_BASE_MS=1` to speed a mock run up now waits real seconds until it is renamed. The [inputs reference](../reference/inputs.md#environment-variables) lists it with the other variables a run reads.
 
+## 19. The sealing key is read at apply time
+
+Every secret family (`actions_secrets`, `dependabot_secrets`, `codespaces_secrets`, `agents_secrets`) now reads `GET .../secrets/public-key` from the first sealed PUT, as environment secrets always did.
+
+- Check mode issues one request fewer per family and never touches the key endpoint.
+- A malformed key fails the first PUT at apply, before its request leaves, where v2 failed the plan in check mode too.
+- The cannot-verify note carries the section label and the shared template: `actions_secrets: Actions secret values cannot be read back from GitHub, so check mode cannot verify them, only that each declared secret exists; apply re-seals and rewrites every declared value on every run`.
+
+## 20. Environment secrets and variables plan through the shared engines
+
+Their lines are the engines' lines now:
+
+| Line | v2 | v3 |
+|---|---|---|
+| A missing declared variable | `environments[prod].variables[X]: missing - declared in the settings file but not on the environment; ...` | `... but not on environment "prod"; ...` |
+| A variable delete | `DELETED undeclared variable "X" from environment "prod"` | `DELETED undeclared variable "X" in environment "prod"` |
+| Two entries naming one variable or secret | `environments: the "prod" entry declares variables that GitHub treats as the same variable ...` | `environments: the settings file declares entries that name the same variable of the "prod" environment: "a" and "A". Keep exactly one entry per resource` (branch policies and protection rules spell theirs the same way) |
+| The secrets cannot-verify note | once per environment that exists | once per environment with declared secrets, the missing environment included, under the `environments[prod].secrets` label |
+
+Repository variable operations (`actions_variables`, `agents_variables`) gain a describe line in failure prose (`creating Actions variable "X"`); nothing else moves.
+
 ## Order of operations
 
 1. Rename any settings file whose path contains a comma, rename `undeclared` to `_undeclared` in every settings file, and move every other underscore key into a YAML comment; the v2 line accepts the old spellings only, so do all three together with the pin move.

src/sections/actions_secrets/actions_secrets.test.ts

@@ -20,7 +20,7 @@ const LIST = "GET /repos/o/r/actions/secrets?per_page=100&page=1";
 const PUBLIC_KEY = "GET /repos/o/r/actions/secrets/public-key";
 const KEY_ROUTE = { data: { key_id: "test-key-id", key: MOCK_SECRETS_PUBLIC_KEY } };
 const CANNOT_VERIFY =
-  "Actions secret values cannot be read back from GitHub, so check mode verifies only that each declared secret exists; apply re-seals and rewrites every declared value on each run";
+  "actions_secrets: Actions secret values cannot be read back from GitHub, so check mode cannot verify them, only that each declared secret exists; apply re-seals and rewrites every declared value on every run";
 
 type Declared = Parameters<typeof actionsSecretsSection.plan>[1];
 
@@ -141,9 +141,9 @@ describe("actions_secrets planning", () => {
       notes: [CANNOT_VERIFY],
       drift: [],
     });
+    // The sealing key is an execution-time read, so check mode issues the list alone.
     expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([
       "GET /repos/o/r/actions/secrets?per_page=100&page=1",
-      "GET /repos/o/r/actions/secrets/public-key",
     ]);
   });
 
@@ -156,9 +156,15 @@ describe("actions_secrets planning", () => {
     ]);
     // Nothing at plan time could have resolved a reference: the resolver is first consulted when a thunk runs.
     const exec = tools({ $A: "value-a", $B: "value-b" });
-    const payloads = result.ops.map((op) =>
-      typeof op.payload === "function" ? sealedPayload({ payload: op.payload(exec) }) : null,
+    const payloads = await Promise.all(
+      result.ops.map(async (op) =>
+        typeof op.payload === "function"
+          ? sealedPayload({ payload: await op.payload(exec) })
+          : null,
+      ),
     );
+    // Both thunks share one key read, issued when the first ran.
+    expect(api.calls.filter((c) => c.path.endsWith("/public-key")).length).toBe(1);
     expect(exec.lookups).toEqual(["$A", "$B"]);
     expect(payloads.map((p) => p?.key_id)).toEqual(["test-key-id", "test-key-id"]);
     expect(payloads.map((p) => unsealSecretValue(p?.encrypted_value ?? ""))).toEqual([
@@ -413,20 +419,24 @@ describe("actions_secrets execution", () => {
 });
 
 describe("actions_secrets sealing key", () => {
-  test("an unusable public key fails the plan loudly, so no operation exists to execute", async () => {
+  test("an unusable public key fails the first PUT before its request leaves; check mode never reads the key", async () => {
     const api = new MockApi({ [LIST]: listOf(), [PUBLIC_KEY]: { data: { key: 42 } } });
-    await expect(plan(api, [{ name: "X", value: "$V" }])).rejects.toThrow(
+    const planned = await plan(api, [{ name: "X", value: "$V" }]);
+    expect(planned.ops.map((op) => op.role)).toEqual(["put"]);
+    await expect(apply(api, [{ name: "X", value: "$V" }], tools({ $V: "v" }))).rejects.toThrow(
       /no usable \{key_id, key\} pair \(key_id is missing\)/,
     );
+    expect(api.mutations()).toEqual([]);
 
     // GitHub requires key_id in the PUT body to route the ciphertext, so an empty one is as unusable as a missing one.
     const emptyId = new MockApi({
       [LIST]: listOf(),
       [PUBLIC_KEY]: { data: { key_id: "", key: MOCK_SECRETS_PUBLIC_KEY } },
     });
-    await expect(plan(emptyId, [{ name: "X", value: "$V" }])).rejects.toThrow(
+    await expect(apply(emptyId, [{ name: "X", value: "$V" }], tools({ $V: "v" }))).rejects.toThrow(
       /no usable \{key_id, key\} pair \(key_id is empty\)/,
     );
+    expect(emptyId.mutations()).toEqual([]);
   });
 });
 

src/sections/actions_secrets/scenarios/actions-secrets-check-drift.yml

@@ -3,8 +3,8 @@ description: >
   Check mode reconciles secrets by existence alone: a declared-but-missing
   secret is drift, the declared values earn one cannot-verify note, an
   undeclared live secret stays a keep-note under the default policy, and no
-  reference is resolved (this scenario deliberately defines no env), so only the list and the sealing key are read - the
-  key is a read the plan closes over for its sealed writes, in every mode.
+  reference is resolved (this scenario deliberately defines no env), so only the list is read; the sealing
+  key is a read the plan closes over for its sealed writes, issued at apply.
 settings:
   actions_secrets:
     - name: DEPLOY_TOKEN

src/sections/actions_variables/actions_variables.test.ts

@@ -99,6 +99,7 @@ describe("actions_variables", () => {
             'actions_variables[DEPLOY_REGION].value: declared "eu-west-1" != live "us-east-1"; apply will set the declared value',
           ],
           change: 'updated Actions variable "DEPLOY_REGION"',
+          describe: 'updating Actions variable "DEPLOY_REGION"',
         },
         {
           role: "create",
@@ -107,6 +108,7 @@ describe("actions_variables", () => {
             "actions_variables[BUILD_MODE]: missing - declared in the settings file but not on the repo; apply will create it",
           ],
           change: 'created Actions variable "BUILD_MODE"',
+          describe: 'creating Actions variable "BUILD_MODE"',
         },
         {
           role: "remove",
@@ -115,6 +117,7 @@ describe("actions_variables", () => {
             "actions_variables[RETIRED_FLAG]: undeclared - not in the settings file, so apply will DELETE it; add it to the settings file to keep it",
           ],
           change: 'DELETED undeclared Actions variable "RETIRED_FLAG"',
+          describe: 'deleting undeclared Actions variable "RETIRED_FLAG"',
         },
       ],
       notes: [],
@@ -168,6 +171,22 @@ describe("actions_variables", () => {
     expect(api.calls).toHaveLength(0);
   });
 
+  test("a passthrough value JSON cannot carry fails the plan at the payload, on a create and on an update alike", async () => {
+    // The loose shape admits any extra key; the engine proves the body plain before it is planned, so
+    // NaN (which JSON would turn into null) never reaches the wire as a silent change of value.
+    const api = new MockApi(listRoute([{ name: "PRESENT", value: "x" }]));
+    const odd = { name: "NEW", value: "v", extra: Number.NaN };
+    await expect(plan(api, [odd as never])).rejects.toThrow(
+      /a planned payload carries a value JSON cannot carry at extra/,
+    );
+    await expect(plan(api, [{ ...odd, name: "PRESENT" } as never])).rejects.toThrow(
+      /a planned payload carries a value JSON cannot carry at extra/,
+    );
+    // The control: a plain extra key rides through.
+    const plain = await plan(api, [{ name: "NEW", value: "v", extra: 42 } as never]);
+    expect(plain.ops.map((op) => op.payload)).toEqual([{ name: "NEW", value: "v", extra: 42 }]);
+  });
+
   test("wrapped _undeclared:keep leaves the undeclared variable as a note, never a DELETE", async () => {
     const api = new MockApi(listRoute(liveVariables));
     const result = await plan(api, {

src/sections/agents_secrets/scenarios/agents-secrets-check-drift.yml

@@ -4,8 +4,8 @@ description: >
   declared-but-missing secret is drift, the declared values earn one
   cannot-verify note, an undeclared live secret stays a keep-note under the
   default policy, and no reference is resolved (this scenario deliberately
-  defines no env), so only the list and the sealing key are read - the
-  key is a read the plan closes over for its sealed writes, in every mode.
+  defines no env), so only the list is read; the sealing
+  key is a read the plan closes over for its sealed writes, issued at apply.
 settings:
   agents_secrets:
     - name: AGENT_TOKEN

src/sections/codespaces_secrets/scenarios/codespaces-secrets-check-drift.yml

@@ -4,8 +4,8 @@ description: >
   declared-but-missing secret is drift, the declared values earn one
   cannot-verify note, an undeclared live secret stays a keep-note under the
   default policy, and no reference is resolved (this scenario deliberately
-  defines no env), so only the list and the sealing key are read - the
-  key is a read the plan closes over for its sealed writes, in every mode.
+  defines no env), so only the list is read; the sealing
+  key is a read the plan closes over for its sealed writes, issued at apply.
 settings:
   codespaces_secrets:
     - name: DOTFILES_PAT

src/sections/contract/endpoints.ts

@@ -84,7 +84,8 @@ export interface GatedReadDecl extends EndpointDeclFields {
   readonly accessGrade: "write";
   readonly alwaysRewrite?: never;
   readonly unverifiable?: never;
-  readonly phase?: never;
+  /** As on a plain GET: "execution" gates the read behind a thunk's token (the Codespaces sealing key). */
+  readonly phase?: "execution";
 }
 
 interface EndpointDeclFields {

src/sections/contract/errors.ts

@@ -60,8 +60,11 @@ export function throwFor(
 ): never {
   // The operation label says WHAT was being done in settings-file terms; the raw method/path keeps the request identifiable.
   //   creating ruleset "x" failed - POST /repos/...: 422 ...
-  const operation = context?.operation ? `${context.operation} failed - ` : "";
-  const cause = `${operation}${method} ${path}: ${error.status} ${error.message}`;
+  //   the token was denied POST /repos/... (creating ruleset "x"): 403 ...
+  const request = `${method} ${path}`;
+  const outcome = `${error.status} ${error.message}`;
+  const cause = `${context?.operation ? `${context.operation} failed - ` : ""}${request}: ${outcome}`;
+  const denied = `${request}${context?.operation ? ` (${context.operation})` : ""}: ${outcome}`;
   if (isRateLimitError(error)) {
     // Secondary rate limits arrive as 403 and must not read as missing permissions.
     throw new Error(
@@ -85,7 +88,7 @@ export function throwFor(
         : sectionGrant(section);
     throw new PermissionDenied(
       section.key,
-      `the token was denied ${cause}${alsoMissing}. To fix, ${grant}${denialHint}`,
+      `the token was denied ${denied}${alsoMissing}. To fix, ${grant}${denialHint}`,
       error.status,
     );
   }

src/sections/dependabot_secrets/scenarios/dependabot-secrets-check-drift.yml

@@ -4,8 +4,8 @@ description: >
   declared-but-missing secret is drift, the declared values earn one
   cannot-verify note, an undeclared live secret stays a keep-note under the
   default policy, and no reference is resolved (this scenario deliberately
-  defines no env), so only the list and the sealing key are read - the
-  key is a read the plan closes over for its sealed writes, in every mode.
+  defines no env), so only the list is read; the sealing
+  key is a read the plan closes over for its sealed writes, issued at apply.
 settings:
   dependabot_secrets:
     - name: REGISTRY_TOKEN

src/sections/environments/branch-policies.ts

@@ -9,6 +9,7 @@ import type { UndeclaredPolicy } from "../../types.js";
 import { liveByIdentity, parseLive } from "../contract/live.js";
 import { type SectionMeta, undeclaredDrift, undeclaredNote } from "../contract/module.js";
 import { hasDrift, plainData } from "../contract/plan.js";
+import { rejectDuplicates } from "../contract/requests.js";
 import { ENDPOINTS, type EnvironmentRestOp, type EnvironmentsRestContext } from "./endpoints.js";
 import type { NestedPlan } from "./nested.js";
 import type { DeploymentBranchPolicyConfig, EnvironmentConfig } from "./schema.js";
@@ -59,23 +60,17 @@ export function livePolicyName(policy: LiveBranchPolicy, envName: string): strin
  * the zod shape (schema.ts), not here, so it fails before any section writes.
  */
 export function validateBranchPolicies(
+  section: SectionMeta,
   env: EnvironmentConfig,
   entries: readonly DeploymentBranchPolicyConfig[],
 ): void {
-  const seen = new Set<string>();
-  const duplicates = new Set<string>();
-  for (const pattern of entries) {
-    if (seen.has(pattern.name)) {
-      duplicates.add(pattern.name);
-    }
-    seen.add(pattern.name);
-  }
-  if (duplicates.size > 0) {
-    throw new Error(
-      `environments: the "${env.name}" entry declares deployment branch polic${duplicates.size === 1 ? "y" : "ies"} ` +
-        `${[...duplicates].map((name) => `"${name}"`).join(", ")} more than once. Keep exactly one entry per pattern`,
-    );
-  }
+  rejectDuplicates(
+    section,
+    entries,
+    (pattern) => pattern.name,
+    (pattern) => pattern.name,
+    `deployment branch policy of the "${env.name}" environment`,
+  );
 }
 
 function createPolicyOp(