refactor(sections): give secretValuesOf a typed signature and drop the container checks it made necessary (#468)

Author: Vivswan

src/sections/contract/module.ts

@@ -756,33 +756,15 @@ export interface DeclaredSecretValue {
 }
 
 /**
- * The secret values of a list section's declared value, one `extract` per entry. The engine hands it zod's output
- * (engine/validate.ts after the shape parse, engine/secrets.ts the validated document), but the erased list
- * declaration and a library caller of sectionModule() reach it as `unknown`, so a malformed container or entry
- * contributes nothing rather than throwing, and the actionable error always comes from validation.
- */
-export function secretValuesOf(
-  declared: unknown,
-  extract: (entry: Readonly<Record<string, unknown>>) => readonly DeclaredSecretValue[],
+ * The secret values of a list section's declared value in either form, one `extract` per entry. Every caller hands
+ * it zod's output (engine/validate.ts after the shape parse, engine/secrets.ts the validated document), so the
+ * entries are the section's own type and a malformed value is validation's to report, never this walk's.
+ */
+export function secretValuesOf<E>(
+  declared: readonly E[] | { readonly entries: readonly E[] },
+  extract: (entry: E) => readonly DeclaredSecretValue[],
 ): DeclaredSecretValue[] {
-  const isWrapper =
-    typeof declared === "object" &&
-    declared !== null &&
-    !Array.isArray(declared) &&
-    Array.isArray((declared as { entries?: unknown }).entries);
-  if (!Array.isArray(declared) && !isWrapper) {
-    return [];
-  }
-  // "keep" is a placeholder: only the entries are read.
-  const { entries } = undeclaredPolicy(
-    declared as readonly unknown[] | UndeclaredPolicyList<unknown>,
-    "keep",
-  );
-  return entries.flatMap((entry) =>
-    typeof entry === "object" && entry !== null && !Array.isArray(entry)
-      ? [...extract(entry as Readonly<Record<string, unknown>>)]
-      : [],
-  );
+  return listEntries(declared).flatMap((entry) => [...extract(entry)]);
 }
 
 /**

src/sections/contract/plan.ts

@@ -156,14 +156,15 @@ export interface ExecTools {
   resolveSecret(reference: string): string;
 }
 
+/** What every member of a bound port is, whatever the helper takes. */
+type Helper = (...args: never[]) => unknown;
+
 /** The runtime shape of the gated ports: the token is discarded, so the gate is the type alone. */
-function gated<T extends object>(bound: T): object {
+function gated<T extends { readonly [H in keyof T]: Helper }>(bound: T): object {
   return Object.fromEntries(
-    Object.entries(bound).map(([name, helper]) => [
+    Object.entries<Helper>(bound).map(([name, helper]) => [
       name,
-      typeof helper === "function"
-        ? (_exec: ExecTools, ...args: unknown[]) => helper(...args)
-        : helper,
+      (_exec: ExecTools, ...args: never[]) => helper(...args),
     ]),
   );
 }

src/sections/environments/index.ts

@@ -12,7 +12,6 @@ import type { SectionFailure } from "../contract/errors.js";
 
 import { liveByIdentity, liveIdentity } from "../contract/live.js";
 import {
-  type DeclaredSecretValue,
   declaredEntries,
   identifiedBy,
   type KeyedListLayering,
@@ -137,21 +136,16 @@ export const environmentsSection = {
   endpoints: ENDPOINTS,
   graphql: GRAPHQL_OPS,
   shape: routed(layeredList(EnvironmentsConfig)),
-  /**
-   * Labels carry the environment: sibling environments can declare same-named secrets.
-   * A malformed container contributes nothing rather than throwing, so the actionable error
-   * always comes from shape validation.
-   */
-  secretValues(declared: unknown): DeclaredSecretValue[] {
-    return secretValuesOf(declared, (entry) => {
-      const env = entry as EnvironmentConfig;
-      const where =
-        typeof env.name === "string" ? `environment "${env.name}"` : "an unnamed environment";
-      return listSecretValues(env.secrets).map(({ label, value }) => ({
-        label: `${label} of ${where}`,
-        value,
-      }));
-    });
+  /** Labels carry the environment: sibling environments can declare same-named secrets. */
+  secretValues(declared) {
+    return secretValuesOf(declared, (env) =>
+      env.secrets === undefined
+        ? []
+        : listSecretValues(env.secrets).map(({ label, value }) => ({
+            label: `${label} of environment "${env.name}"`,
+            value,
+          })),
+    );
   },
   validate(desired) {
     const issues = IDENTITY.validate(desired);

src/sections/shared/list-section.ts

@@ -27,7 +27,15 @@ import { snapshotList } from "./list-section-snapshot.js";
 import { identityClaims, pathOf, valueAt } from "./list-section-write.js";
 import { knobbed, routed } from "./schema-helpers.js";
 
-function secretValuesFor(decl: ErasedDecl<string>, declared: unknown): DeclaredSecretValue[] {
+/**
+ * valueAt() walks the erased entry, so a declared secret field reads as `unknown`: absent on an entry that leaves
+ * the optional field out, a string otherwise, like the identity beside it. The narrowing is that walk's, not a
+ * second proof of the shape.
+ */
+function secretValuesFor(
+  decl: ErasedDecl<string>,
+  declared: ErasedDeclared,
+): DeclaredSecretValue[] {
   const fields = decl.secrets ?? [];
   return secretValuesOf(declared, (entry) =>
     fields.flatMap((field) => {
@@ -66,7 +74,10 @@ export function listSection<
     shape: routed(knobbed(decl.entry)),
     ...(decl.secrets === undefined
       ? {}
-      : { secretValues: (declared: Declared<K>) => secretValuesFor(erased, declared) }),
+      : {
+          secretValues: (declared: Declared<K>) =>
+            secretValuesFor(erased, declared as unknown as ErasedDeclared),
+        }),
     layering: {
       keys: (entry) => identityClaims(erased.identity, entry),
       keyField: decl.identity.field,

src/sections/shared/secrets-engine.ts

@@ -93,17 +93,12 @@ export function secretOps<
 }
 
 /** Each value is labelled with its entry's secret NAME so a validation error can point at it. */
-export function listSecretValues(declared: unknown): DeclaredSecretValue[] {
-  return secretValuesOf(declared, (entry) => {
-    if (typeof entry.value !== "string") {
-      return [];
-    }
-    const label =
-      typeof entry.name === "string"
-        ? `the secret entry "${entry.name}"`
-        : "an unnamed secret entry";
-    return [{ label, value: entry.value }];
-  });
+export function listSecretValues(
+  declared: SecretEntry[] | UndeclaredPolicyList<SecretEntry>,
+): DeclaredSecretValue[] {
+  return secretValuesOf(declared, (entry) => [
+    { label: `the secret entry "${entry.name}"`, value: entry.value },
+  ]);
 }
 
 export interface SealingKey {

test/sections/environments/environments.test.ts

@@ -556,23 +556,16 @@ describe("environments nested secrets validation and shape", () => {
     );
   });
 
-  test("secretValues walks every entry's secrets list and survives malformed containers", () => {
-    // The double cast feeds secretValues a pre-validation document slice on purpose: its contract is defensiveness against any merged value.
+  test("secretValues walks every entry's secrets list in either nested form and labels each by its environment", () => {
     const values = environmentsSection.secretValues?.([
       { name: "a", secrets: [{ name: "X", value: "$X" }] },
       { name: "b", secrets: { entries: [{ name: "Y", value: "$Y" }] } },
       { name: "c" },
-      { name: "d", secrets: "garbage" },
-      "not-an-entry",
-    ] as unknown as EnvironmentConfig[]);
+    ]);
     expect(values).toEqual([
       { label: 'the secret entry "X" of environment "a"', value: "$X" },
       { label: 'the secret entry "Y" of environment "b"', value: "$Y" },
     ]);
-    // A non-list section value contributes nothing (validation reports it).
-    expect(
-      environmentsSection.secretValues?.({ not: "a list" } as unknown as EnvironmentConfig[]),
-    ).toEqual([]);
   });
 });
 

test/sections/webhooks/webhooks.test.ts

@@ -125,14 +125,6 @@ describe("webhooks secretValues", () => {
       }),
     ).toEqual([{ label: 'the webhook "https://a.test" config.secret', value: "$B" }]);
   });
-
-  test("malformed containers return [] and leave the error to validation", () => {
-    // The extractor faces any merged value, so the double cast feeds it a pre-validation value on purpose; validation is where the user gets the
-    // message.
-    for (const malformed of [null, "hooks", 42, { _undeclared: "keep" }, [null, "x"]]) {
-      expect(webhooksSection.secretValues?.(malformed as unknown as WebhookConfig[])).toEqual([]);
-    }
-  });
 });
 
 describe("webhooks plan", () => {