diff --git a/docs/reference/coverage.md b/docs/reference/coverage.md index 654b8fb1..c653aab1 100644 --- a/docs/reference/coverage.md +++ b/docs/reference/coverage.md @@ -227,6 +227,7 @@ The side-by-side comparison with the Probot Settings app lives in the migration - PUT /repos/{owner}/{repo}/environments/{name} passthrough; check mode flattens GET's protection_rules[] back into the PUT shape. - A live wait timer or reviewers list the entry omits is drift, and apply refuses the PUT (the [replace-write rule](semantics.md)). - Undeclared environments are left untouched. +- A key the entry shape does not declare rides the PUT. One the GET reports and the PUT does not take (id, node_id, url, html_url, created_at, updated_at, protection_rules) is named by a note in every run. Any other, unless declared null or as an empty string, is named by a note once the environment exists and its GET does not echo the key. - In check mode against a missing environment the declared variables, secrets, patterns, and protection rules cannot be listed, so notes say they are unverifiable until it exists; against a live environment whose custom_branch_policies flag is off, the patterns earn the same note while the flag drift comes from the environment diff. - Snapshot lists the environments (GET /repos/{owner}/{repo}/environments) and reads each one back as an entry: the PUT-shape settings off its GET body, and every nested list that has live items in its wrapped form under that list's default policy. - The snapshot writes variables with their values, branch-policy patterns while the custom_branch_policies flag is on, and the enabled protection rules by App slug; those two lists sit behind the Actions grant: under on-missing-permission: warn a denial leaves the key out with a note and the rest of the environment reads back, under fail it fails the section. @@ -236,6 +237,7 @@ The side-by-side comparison with the Probot Settings app lives in the migration - A declared per-environment `variables` key reconciles that environment's Actions variables AFTER the PUT, through GET/POST /repos/{owner}/{repo}/environments/{name}/variables and PATCH/DELETE .../variables/{name}: create missing, update divergent values, and delete undeclared ones by default (the wrapped `_undeclared: keep` form keeps them as notes). - Names match case-insensitively, values are plain text by design. +- A declared created_at or updated_at, which the GET reports and neither write takes, is named by a note in every run. **Environment secrets** (`environments`) @@ -248,6 +250,7 @@ The side-by-side comparison with the Probot Settings app lives in the migration - A declared per-environment `deployment_branch_policies` key reconciles that environment's custom branch-policy patterns through GET/POST .../environments/{name}/deployment-branch-policies and DELETE .../deployment-branch-policies/{branch_policy_id}: create missing patterns, delete undeclared ones by default (`_undeclared: keep` softens to notes), and replace a matching pattern whose type differs. - Type is immutable upstream, so the change is delete + recreate; the upstream PUT is deliberately unused - its body is the name alone, and the name is the pattern's identity, so it can never help reconciliation. +- A pattern key outside name and type rides the create call and no run compares it, so it adds no drift. One the GET reports (id, node_id) is named by a note in every run. Any other, unless declared null or as an empty string, is named by a note once the live pattern exists and does not echo the key. - Declaring the key requires the singular `deployment_branch_policy` sibling with custom_branch_policies: true (rejected upfront otherwise, since the pattern writes would 404 only after the environment PUT landed). - These endpoints sit outside the Environments PAT permission: the list read needs Actions read, the writes need Administration write. diff --git a/docs/sections/environments.docs.yml b/docs/sections/environments.docs.yml index 1e6837c3..0195b7e5 100644 --- a/docs/sections/environments.docs.yml +++ b/docs/sections/environments.docs.yml @@ -24,6 +24,11 @@ coverage: A live wait timer or reviewers list the entry omits is drift, and apply refuses the PUT (the [replace-write rule](semantics.md)). - Undeclared environments are left untouched. + - >- + A key the entry shape does not declare rides the PUT. One the GET reports and the PUT does not + take (id, node_id, url, html_url, created_at, updated_at, protection_rules) is named by a + note in every run. Any other, unless declared null or as an empty string, is named by a note + once the environment exists and its GET does not echo the key. - >- In check mode against a missing environment the declared variables, secrets, patterns, and protection rules cannot be listed, so notes say they are unverifiable until it exists; against @@ -53,6 +58,9 @@ coverage: PATCH/DELETE .../variables/{name}: create missing, update divergent values, and delete undeclared ones by default (the wrapped `_undeclared: keep` form keeps them as notes). - Names match case-insensitively, values are plain text by design. + - >- + A declared created_at or updated_at, which the GET reports and neither write takes, is named + by a note in every run. - area: "[Environment secrets](https://docs.github.com/en/rest/actions/secrets)" keys: secrets endpoints: [listSecrets, secretsPublicKey, putSecret, removeSecret] @@ -84,6 +92,11 @@ coverage: Type is immutable upstream, so the change is delete + recreate; the upstream PUT is deliberately unused - its body is the name alone, and the name is the pattern's identity, so it can never help reconciliation. + - >- + A pattern key outside name and type rides the create call and no run compares it, so it adds + no drift. One the GET reports (id, node_id) is named by a note in every run. Any other, unless + declared null or as an empty string, is named by a note once the live pattern exists and does + not echo the key. - >- Declaring the key requires the singular `deployment_branch_policy` sibling with custom_branch_policies: true (rejected upfront otherwise, since the pattern writes would 404 diff --git a/src/engine/diff.ts b/src/engine/diff.ts index fbff3aaf..c8072a38 100644 --- a/src/engine/diff.ts +++ b/src/engine/diff.ts @@ -581,11 +581,18 @@ export function phantomKeys(desired: Record, live: unknown): st return phantomPaths(deltas(desired, live)); } -export function phantomNote(prefix: string, keys: string[], noun: string, rewrite: string): string { +/** `rewrite` and `tail` finish "so if GitHub ignores it ...": the default tail fits a write the comparison gates, which re-runs; a write nothing gates passes what happens instead. */ +export function phantomNote( + prefix: string, + keys: string[], + noun: string, + rewrite: string, + tail = "on every apply without converging", +): string { const list = keys.map((k) => `"${k}"`).join(", "); const count = keys.length; return ( `${prefix}: declared ${agree(count, "key", "keys")} ${list} ${agree(count, "does", "do")} not exist on the live ${noun}, ` + - `so if GitHub ignores ${agree(count, "it", "them")} ${rewrite} on every apply without converging. Fix the key name, or remove it from the settings file` + `so if GitHub ignores ${agree(count, "it", "them")} ${rewrite} ${tail}. Fix the key name, or remove it from the settings file` ); } diff --git a/src/sections/environments/branch-policies.ts b/src/sections/environments/branch-policies.ts index 2c4c4e39..2e6ef09b 100644 --- a/src/sections/environments/branch-policies.ts +++ b/src/sections/environments/branch-policies.ts @@ -5,7 +5,7 @@ import { err, ok, Result, safeTry } from "neverthrow"; import { z } from "zod"; -import { subsetDiff } from "../../engine/diff.js"; +import { phantomKeys, phantomNote, subsetDiff } from "../../engine/diff.js"; import type { UndeclaredPolicy } from "../../types.js"; import { type DeclaredIssue, duplicateFieldIssues } from "../contract/declared.js"; import { missingDrift, undeclaredDrift, undeclaredNote } from "../contract/drift.js"; @@ -13,6 +13,7 @@ import type { SectionFailure } from "../contract/errors.js"; import { liveByIdentity, liveIdentity } from "../contract/live.js"; import type { SectionMeta } from "../contract/module.js"; import { hasDrift, type Read } from "../contract/plan.js"; +import { reportedKeyNote } from "../shared/reported-key-note.js"; import { type EnvironmentRestOp, type EnvironmentsRestContext, @@ -20,6 +21,7 @@ import { } from "./endpoints.js"; import type { LiveEnvironmentBody } from "./index.js"; import type { NestedPlan } from "./nested.js"; +import { BRANCH_POLICY_REPORTED_KEYS } from "./reported-keys.js"; import type { DeploymentBranchPolicyConfig } from "./schema.js"; /** "delete" like the nested variables list: patterns are readable, recreatable configuration. */ @@ -41,6 +43,10 @@ function livePolicyType(policy: LiveBranchPolicy): string { const POLICY = { list: "deployment branch-policy", entry: "policy" }; +function patternLabel(envName: string, name: string): string { + return `environments[${envName}].deployment_branch_policies[${name}]`; +} + function livePolicyId(policy: LiveBranchPolicy, envName: string): Result { if (policy.id === undefined) { return err(unreconcilable(POLICY, envName, "an id")); @@ -134,7 +140,7 @@ function patternOps( existing: LiveBranchPolicy | undefined, hidden: boolean, ): Result { - const label = `environments[${envName}].deployment_branch_policies[${pattern.name}]`; + const label = patternLabel(envName, pattern.name); if (!existing) { return ok([ { @@ -180,6 +186,38 @@ function patternOps( ]); } +/** What follows for a declared pattern key GitHub does not take, in the two halves both of its notes end on. */ +const KEY_OUTCOME = ["the pattern is unaffected", "and no check reports that as drift"] as const; + +/** + * The notes one declared pattern earns for keys outside its shape. Such a key rides the create call alone and no + * run compares it, so no drift line can name it: a key GitHub only reports is named from the declaration alone, + * any other once the live pattern exists and does not echo it. + */ +function undeclaredKeyNotes( + envName: string, + pattern: DeploymentBranchPolicyConfig, + existing: LiveBranchPolicy | undefined, +): string[] { + const label = patternLabel(envName, pattern.name); + const { name: _name, type: _type, ...extra } = pattern as Record; + const reported = reportedKeyNote(label, extra, { + keys: BRANCH_POLICY_REPORTED_KEYS, + write: "the POST does not take", + outcome: `so ${KEY_OUTCOME.join(" ")}`, + }); + const notes = [...reported.notes]; + if (existing === undefined) { + return notes; + } + + const unechoed = phantomKeys(extra, existing).filter((key) => !reported.keys.includes(key)); + if (unechoed.length > 0) { + notes.push(phantomNote(label, unechoed, "deployment branch policy", ...KEY_OUTCOME)); + } + return notes; +} + /** A keep-note or the delete for each live pattern the list does not name. */ function undeclaredPolicyOps( envName: string, @@ -211,7 +249,7 @@ function undeclaredPolicyOps( }, drift: [ undeclaredDrift(BRANCH_POLICIES_DEFAULT_POLICY, { - label: `environments[${envName}].deployment_branch_policies[${name}]`, + label: patternLabel(envName, name), action: "DELETE it", }), ], @@ -250,8 +288,10 @@ export async function planBranchPolicies( const declared = new Set(entries.map((pattern) => pattern.name)); for (const pattern of entries) { - const ops = yield* patternOps(envName, pattern, liveByName.get(pattern.name), hidden); + const existing = liveByName.get(pattern.name); + const ops = yield* patternOps(envName, pattern, existing, hidden); planned.ops.push(...ops); + planned.notes.push(...undeclaredKeyNotes(envName, pattern, existing)); } const undeclared = yield* undeclaredPolicyOps(envName, liveByName, declared, policy); diff --git a/src/sections/environments/index.ts b/src/sections/environments/index.ts index 5558cdb9..6601fa0c 100644 --- a/src/sections/environments/index.ts +++ b/src/sections/environments/index.ts @@ -16,6 +16,7 @@ import { liveByIdentity, liveIdentity } from "../contract/live.js"; import type { SectionMeta, SectionModule } from "../contract/module.js"; import type { SectionPermission } from "../contract/permissions.js"; import { hasDrift } from "../contract/plan.js"; +import { reportedKeyNote } from "../shared/reported-key-note.js"; import { layeredList, routed } from "../shared/schema-helpers.js"; import { upperKey } from "../shared/secrets-and-variables/named-scope.js"; import { listSecretValues } from "../shared/secrets-and-variables/secrets-engine.js"; @@ -38,6 +39,7 @@ import { planPinned, snapshotPins, } from "./pins.js"; +import { ENVIRONMENT_REPORTED_KEYS } from "./reported-keys.js"; import { EnvironmentConfig, EnvironmentsConfig, environmentKey } from "./schema.js"; import { sharedSecretNotes, snapshotNested, withPins } from "./snapshot.js"; @@ -164,21 +166,27 @@ export function flattenEnvironment(live: LiveEnvironmentBody): Record, live: LiveEnvironmentBody | undefined, ): { drift: string[]; omitted: string[]; notes: string[] } { + const reported = reportedKeyNote(label, settings, { + keys: ENVIRONMENT_REPORTED_KEYS, + write: "the PUT does not take", + outcome: "so GitHub answers this environment's PUT with a 422 whenever apply sends one", + }); + const notes = [...reported.notes]; if (live === undefined) { - return { drift: [missingDrift(label)], omitted: [], notes: [] }; + return { drift: [missingDrift(label)], omitted: [], notes }; } const flattened = flattenEnvironment(live); @@ -187,10 +195,11 @@ function environmentDrift( sweep: replaceSweep(EnvironmentConfig), }).map((delta) => renderDelta(label, delta)); const phantom = phantomKeys(settings, flattened).filter( - (key) => !Object.hasOwn(EnvironmentConfig.shape, key), + (key) => !Object.hasOwn(EnvironmentConfig.shape, key) && !reported.keys.includes(key), ); - const notes = - phantom.length > 0 ? [phantomNote(label, phantom, "environment", "this PUT will re-run")] : []; + if (phantom.length > 0) { + notes.push(phantomNote(label, phantom, "environment", "this PUT will re-run")); + } return { drift: [...subsetDiff(settings, flattened, label), ...omitted], omitted, notes }; } diff --git a/src/sections/environments/reported-keys.ts b/src/sections/environments/reported-keys.ts new file mode 100644 index 00000000..b4fe631e --- /dev/null +++ b/src/sections/environments/reported-keys.ts @@ -0,0 +1,50 @@ +/** + * The keys GitHub only reports on an environment entry and on a branch-policy pattern: its GET returns them and + * the write does not take them. The open shapes in schema.ts still parse one, so a plan names it in a note + * (../shared/reported-key-note.ts). Each list is pinned both ways to the vendored spec, by its satisfies and its + * MustBeNever. + */ + +import type { components, operations } from "@octokit/openapi-types"; +import type { MustBeNever } from "../../types.js"; + +type EnvironmentPutBody = NonNullable< + NonNullable< + operations["repos/create-or-update-environment"]["requestBody"] + >["content"]["application/json"] +>; + +type ReportedEnvironmentKey = Exclude< + keyof components["schemas"]["environment"], + keyof EnvironmentPutBody | "name" +>; + +/** On an environment entry; `name` addresses the PUT, so it is no reported key. */ +export const ENVIRONMENT_REPORTED_KEYS = [ + "id", + "node_id", + "url", + "html_url", + "created_at", + "updated_at", + "protection_rules", +] as const satisfies readonly ReportedEnvironmentKey[]; + +type _EnvironmentKeysComplete = MustBeNever< + Exclude +>; + +type ReportedBranchPolicyKey = Exclude< + keyof components["schemas"]["deployment-branch-policy"], + keyof components["schemas"]["deployment-branch-policy-name-pattern-with-type"] +>; + +/** On a pattern entry, whose only write is the create POST. */ +export const BRANCH_POLICY_REPORTED_KEYS = [ + "id", + "node_id", +] as const satisfies readonly ReportedBranchPolicyKey[]; + +type _BranchPolicyKeysComplete = MustBeNever< + Exclude +>; diff --git a/src/sections/environments/schema.ts b/src/sections/environments/schema.ts index 8c0e0e6a..64448203 100644 --- a/src/sections/environments/schema.ts +++ b/src/sections/environments/schema.ts @@ -43,7 +43,10 @@ const MAX_REVIEWERS = 6; /** GitHub's cap on pinned environments per repository. */ export const MAX_PINNED_ENVIRONMENTS = 10; -/** A branch pattern entry; an absent type is GitHub's own default, "branch". */ +/** + * A branch pattern entry; an absent type is GitHub's own default, "branch". + * COMPAT(v4): a key GitHub only reports (BRANCH_POLICY_REPORTED_KEYS in reported-keys.ts) still parses, rides the create POST, and is named by a note; delete the note and refuse the key here, the rule the actions section's reported-only sweep applies. + */ export const DeploymentBranchPolicyConfig = open({ name: z.string(), type: z.enum(DEPLOYMENT_BRANCH_POLICY_TYPES).optional(), @@ -102,7 +105,10 @@ const DeploymentBranchPolicy = open(DeploymentBranchPolicyFlags.unwrap().shape) ), }); -/** One environment entry: the PUT body's keys, plus the routed scalar and the nested lists nested.ts strips before the PUT. */ +/** + * One environment entry: the PUT body's keys, plus the routed scalar and the nested lists nested.ts strips before the PUT. + * COMPAT(v4): a key GitHub only reports (ENVIRONMENT_REPORTED_KEYS in reported-keys.ts) still parses, rides the PUT, and is named by a note; delete the note and refuse the key here, the rule the actions section's reported-only sweep applies. + */ export const EnvironmentConfig = open({ name: z.string(), // Routed (see EnvironmentRoutedScalars): stripped from the PUT body and applied through the diff --git a/src/sections/shared/reported-key-note.ts b/src/sections/shared/reported-key-note.ts new file mode 100644 index 00000000..dd63bcab --- /dev/null +++ b/src/sections/shared/reported-key-note.ts @@ -0,0 +1,38 @@ +/** + * The note for declared keys GitHub only reports: its GET returns them and no write takes them. An open shape still + * parses one, so its planner names it here, from the declaration alone, whatever the live side holds. + */ + +import { agree } from "../../text.js"; + +/** One holder's reported keys and the words its note puts around them. */ +interface ReportedKeys { + /** The keys the holder's GET reports and its write does not take. */ + readonly keys: readonly string[]; + + /** The clause each key completes: "the PUT does not take", "neither the POST nor the PATCH takes". */ + readonly write: string; + + /** What follows for a declared one, from "so" on. */ + readonly outcome: string; +} + +/** The reported keys `declared` holds, and the one note naming them; no note when it holds none. */ +export function reportedKeyNote( + label: string, + declared: object, + reported: ReportedKeys, +): { keys: readonly string[]; notes: string[] } { + const keys = reported.keys.filter((key) => Object.hasOwn(declared, key)); + if (keys.length === 0) { + return { keys, notes: [] }; + } + + const list = keys.map((key) => `"${key}"`).join(", "); + const count = keys.length; + const them = agree(count, "it", "them"); + const note = + `${label}: ${list} ${agree(count, "is a value", "are values")} GitHub reports, not ${agree(count, "a setting", "settings")} it accepts ` + + `(the GET returns ${them}, ${reported.write} ${them}), ${reported.outcome}; remove ${them} from the settings file`; + return { keys, notes: [note] }; +} diff --git a/src/sections/shared/schema-helpers.ts b/src/sections/shared/schema-helpers.ts index 32364e69..82cf61d6 100644 --- a/src/sections/shared/schema-helpers.ts +++ b/src/sections/shared/schema-helpers.ts @@ -351,7 +351,10 @@ export function sealedSecretConfig(id: string) { return open({ name: secretName, value: z.string() }).meta({ id }); } -/** A repository-scope plain-text variable entry; the environments section's nested list is the same shape. */ +/** + * A repository-scope plain-text variable entry; the environments section's nested list is the same shape. + * COMPAT(v4): a declared created_at or updated_at still parses, rides the write, and is named by a note; delete the note in secrets-and-variables/variables-engine.ts and refuse both here, the rule the actions section's reported-only sweep applies. + */ export function variableConfig(id: string) { return open({ name: variableName, value: variableValue }).meta({ id }); } diff --git a/src/sections/shared/secrets-and-variables/variables-engine.ts b/src/sections/shared/secrets-and-variables/variables-engine.ts index de018c68..dfb59d30 100644 --- a/src/sections/shared/secrets-and-variables/variables-engine.ts +++ b/src/sections/shared/secrets-and-variables/variables-engine.ts @@ -4,13 +4,16 @@ * (../../environments/nested.ts) plan through it; the frame is ./named-scope.ts. */ +import type { components, operations } from "@octokit/openapi-types"; import type { Result } from "neverthrow"; import { z } from "zod"; import { phantomKeys, phantomNote, subsetDiff } from "../../../engine/diff.js"; +import type { MustBeNever } from "../../../types.js"; import { valueDrift } from "../../contract/drift.js"; import type { SectionFailure } from "../../contract/errors.js"; import type { SectionMeta } from "../../contract/module.js"; import { type PlainData, paramsWith, type SectionPlan } from "../../contract/plan.js"; +import { reportedKeyNote } from "../reported-key-note.js"; import { type AnyPlannedOp, missingNamedDrift, @@ -25,6 +28,30 @@ export type LiveVariable = z.infer; type PlainPayload = PlainData; +/** A field the variable GET reports that neither write takes; every scope serves the one variable object. */ +type ReportedVariableKey = Exclude< + keyof components["schemas"]["actions-variable"], + keyof operations["actions/create-repo-variable"]["requestBody"]["content"]["application/json"] +>; + +/** Named by a note in every run; the satisfies and the MustBeNever below pin the list both ways to the vendored spec. */ +const REPORTED_KEYS = [ + "created_at", + "updated_at", +] as const satisfies readonly ReportedVariableKey[]; + +type _ReportedKeysComplete = MustBeNever< + Exclude +>; + +/** The reported keys and the words a variable's note puts around them. */ +const REPORTED = { + keys: REPORTED_KEYS, + write: "neither the POST nor the PATCH takes", + outcome: + "so a declared value that differs from the live one drifts on every check and its update re-runs on every apply without converging", +}; + /** The index signature types the passthrough fields as plain data, so spreading them into a body needs no cast. */ export interface VariableEntry { readonly name: string; @@ -98,6 +125,8 @@ export function planVariables< const label = `${scope.label}[${variable.name}]`; const existing = liveOf(variable); const { name: _name, value: _value, ...extraKeys } = variable; + const reported = reportedKeyNote(label, extraKeys, REPORTED); + notes.push(...reported.notes); if (!existing) { ops.push( scope.create({ @@ -127,7 +156,9 @@ export function planVariables< if (first === undefined) { continue; } - const phantom = phantomKeys(extraKeys, existing); + const phantom = phantomKeys(extraKeys, existing).filter( + (key) => !reported.keys.includes(key), + ); if (phantom.length > 0) { notes.push(phantomNote(label, phantom, "variable", "this update will re-run")); } diff --git a/test/sections/actions_variables/actions_variables.test.ts b/test/sections/actions_variables/actions_variables.test.ts index 3b572acd..44124f5c 100644 --- a/test/sections/actions_variables/actions_variables.test.ts +++ b/test/sections/actions_variables/actions_variables.test.ts @@ -278,4 +278,24 @@ describe("actions_variables", () => { ), ]); }); + + test.each([ + ["the variable is missing", []], + ["the live variable carries no timestamp", [{ name: "DEPLOY_REGION", value: "us-east-1" }]], + ] as Array<[string, Array<{ name: string; value: string }>]>)( + "a declared created_at, which GitHub only reports, is named once when %s", + async (_state, live) => { + // The engine is shared with the environments section's nested variables: without the note a copied timestamp drifts on every check unexplained. + const variable = { + name: "DEPLOY_REGION", + value: "us-east-1", + created_at: "2026-01-01T00:00:00Z", + }; + const result = await plan(new MockApi(listRoute(live)), [variable as never]); + expect(result.notes).toEqual([ + 'actions_variables[DEPLOY_REGION]: "created_at" is a value GitHub reports, not a setting it accepts ' + + "(the GET returns it, neither the POST nor the PATCH takes it), so a declared value that differs from the live one drifts on every check and its update re-runs on every apply without converging; remove it from the settings file", + ]); + }, + ); }); diff --git a/test/sections/environments/environments.test.ts b/test/sections/environments/environments.test.ts index 50e02eef..e3cb2e0b 100644 --- a/test/sections/environments/environments.test.ts +++ b/test/sections/environments/environments.test.ts @@ -7,6 +7,7 @@ import { flattenEnvironment, } from "../../../src/sections/environments/index.js"; import { + type DeploymentBranchPolicyConfig, EnvironmentConfig, type EnvironmentVariableConfig, } from "../../../src/sections/environments/schema.js"; @@ -694,6 +695,110 @@ describe("environments deployment branch policies check mode", () => { }); }); +describe("environments keys outside the entry shapes", () => { + const ENV = "GET /repos/o/r/environments/prod"; + + test.each([ + ["the environment is missing", {}, 1], + ["the live id equals the declared one", { [ENV]: liveEnv("prod", { id: 1 }) }, 1], + ["the live id differs", { [ENV]: liveEnv("prod", { id: 99 }) }, 1], + ["the live body carries no id", { [ENV]: liveEnv("prod") }, 1], + // A reported key declared null is not inert: it drifts against the live id and rides the PUT. + ["it is declared null", { [ENV]: liveEnv("prod", { id: 99 }) }, null], + ] as Array<[string, Record, number | null]>)( + "an entry's id, which GitHub only reports, is named once when %s", + async (_state, routes, id) => { + // Without the note an id equal to the live one reads clean in every run, until another key drifts and the PUT that carries it answers 422. + const checked = await check(new MockApi(routes), [{ name: "prod", id } as EnvironmentConfig]); + expect(checked.notes).toEqual([ + 'environments[prod]: "id" is a value GitHub reports, not a setting it accepts (the GET returns it, the PUT does not take it), so GitHub answers this environment\'s PUT with a 422 whenever apply sends one; remove it from the settings file', + ]); + }, + ); + + test("two reported keys on one entry share one note, worded for several", async () => { + // Without it a note per key, or the one-key grammar over two keys, would print unnoticed. + const checked = await check(new MockApi({}), [ + { name: "prod", node_id: "EN_1", html_url: "https://github.com/o/r" } as EnvironmentConfig, + ]); + expect(checked.notes).toEqual([ + 'environments[prod]: "node_id", "html_url" are values GitHub reports, not settings it accepts ' + + "(the GET returns them, the PUT does not take them), so GitHub answers this environment's PUT with a 422 whenever apply sends one; remove them from the settings file", + ]); + }); + + test("control: a key the entry shape declares draws no note", async () => { + const checked = await check(new MockApi({ [ENV]: liveEnv("prod") }), [ + { name: "prod", wait_timer: 5 }, + ]); + expect(checked.drift).toEqual(["environments[prod].wait_timer: 5 != 0"]); + expect(checked.notes).toEqual([]); + }); + + test.each([ + ["the variable is missing", []], + [ + "the live variable carries another timestamp", + [{ name: "X", value: "v", updated_at: "2026-07-01T00:00:00Z" }], + ], + [ + "the live variable carries the same timestamp", + [{ name: "X", value: "v", updated_at: "2026-01-01T00:00:00Z" }], + ], + ["the live variable carries no timestamp", [{ name: "X", value: "v" }]], + ] as Array<[string, Array<{ name: string; value: string }>]>)( + "a variable's updated_at, which GitHub only reports, is named once when %s", + async (_state, live) => { + // Without the note a timestamp that differs drifts on every check and re-sends its PATCH on every apply, and one that matches reads clean, with no line saying why. + const api = new MockApi({ [ENV]: liveEnv("prod"), [VARIABLES_LIST]: variablesBody(live) }); + const variable = { name: "X", value: "v", updated_at: "2026-01-01T00:00:00Z" }; + const checked = await check(api, [ + { name: "prod", variables: [variable as EnvironmentVariableConfig] }, + ]); + expect(checked.notes).toEqual([ + 'environments[prod].variables[X]: "updated_at" is a value GitHub reports, not a setting it accepts ' + + "(the GET returns it, neither the POST nor the PATCH takes it), so a declared value that differs from the live one drifts on every check and its update re-runs on every apply without converging; remove it from the settings file", + ]); + }, + ); + + test.each([ + ["the pattern is missing", []], + ["the pattern is live under another id", [{ id: 41, name: "main", type: "branch" }]], + ["the live pattern carries no id", [{ name: "main", type: "branch" }]], + ] as Array<[string, Array<{ id?: number; name: string; type: string }>]>)( + "a pattern's id, which GitHub only reports, is named once when %s", + async (_state, live) => { + // Without the note the id rides the create call, is never compared, and the check reads clean. + const api = new MockApi({ + [ENV]: liveProdWithFlag(true), + [POLICIES_LIST]: policiesBody(live), + }); + const pattern = { name: "main", id: 7 } as DeploymentBranchPolicyConfig; + const checked = await check(api, [envWithPolicies([pattern])]); + expect(checked.notes).toEqual([ + 'environments[prod].deployment_branch_policies[main]: "id" is a value GitHub reports, not a setting it accepts ' + + "(the GET returns it, the POST does not take it), so the pattern is unaffected and no check reports that as drift; remove it from the settings file", + ]); + }, + ); + + test("a pattern key the live pattern does not echo is named beside a clean check", async () => { + // A typo of `type`: GitHub creates a branch pattern, nothing compares the key afterwards, and without the note no run says so. + const api = new MockApi({ + [ENV]: liveProdWithFlag(true), + [POLICIES_LIST]: policiesBody([{ id: 41, name: "v*", type: "branch" }]), + }); + const pattern = { name: "v*", typ: "tag" } as DeploymentBranchPolicyConfig; + const checked = await check(api, [envWithPolicies([pattern])]); + expect(checked.drift).toEqual([]); + expect(checked.notes).toEqual([ + 'environments[prod].deployment_branch_policies[v*]: declared key "typ" does not exist on the live deployment branch policy, ' + + "so if GitHub ignores it the pattern is unaffected and no check reports that as drift. Fix the key name, or remove it from the settings file", + ]); + }); +}); + describe("environments parse rules", () => { test("every combination GitHub 422s or never reads back is refused at parse time, naming the key and the fix", () => { // Shape rules, not a plan() hook, so upfront validation rejects the document in both modes before any section writes (the apply-mode preflight diff --git a/test/sections/environments/scenarios/environment-branch-policies-unknown-key-noted.yml b/test/sections/environments/scenarios/environment-branch-policies-unknown-key-noted.yml new file mode 100644 index 00000000..fa0ac780 --- /dev/null +++ b/test/sections/environments/scenarios/environment-branch-policies-unknown-key-noted.yml @@ -0,0 +1,46 @@ +# A misspelled key on a pattern entry (typ for type) passes through: the create +# call carried it, GitHub ignored it, and the pattern lives as a branch pattern. +# No run compares a pattern's extra key, so the check stays clean and the note +# is the one line that names the key. Nothing is written. +name: environment-branch-policies-unknown-key-noted +settings: + environments: + - name: production + deployment_branch_policy: + protected_branches: false + custom_branch_policies: true + deployment_branch_policies: + - name: v* + typ: tag +inputs: + mode: check +live_state: + environments: + production: + name: production + protection_rules: [] + deployment_branch_policy: + protected_branches: false + custom_branch_policies: true + environment_branch_policies: + production: + - id: 4001 + name: v* + type: branch +token_permissions: + environments: read + actions: read +expect: + exit_code: 0 + result: clean + outcomes: + environments: clean + stdout_contains: + - 'environments[production].deployment_branch_policies[v*]: declared key "typ" does not exist on the live deployment branch policy' + - "so if GitHub ignores it the pattern is unaffected and no check reports that as drift. Fix the key name, or remove it from the settings file" + never: + - "PUT /repos/{repo}/environments" + - "POST /repos/{repo}/environments/production/deployment-branch-policies" + - "DELETE /repos/{repo}/environments" + requests_contain: + - "GET /repos/e2e-owner/e2e-repo/environments/production/deployment-branch-policies" diff --git a/test/sections/environments/scenarios/environments-apply-refused-entry-reports-unattempted.yml b/test/sections/environments/scenarios/environments-apply-refused-entry-reports-unattempted.yml new file mode 100644 index 00000000..21e75281 --- /dev/null +++ b/test/sections/environments/scenarios/environments-apply-refused-entry-reports-unattempted.yml @@ -0,0 +1,50 @@ +# A failed operation ends its section's plan, since a later operation may +# depend on an earlier one, and the run names what that left unsent. +# +# Here the first environment's PUT is refused: it carries `id`, a key GitHub +# only reports, and the environment PUT takes no key outside its own. The second +# environment's variable and secret are then never written. One warning counts +# them, and each gets its own "not attempted" line, in stdout and in the summary. +name: environments-apply-refused-entry-reports-unattempted +settings: + environments: + - name: production + id: 1 + - name: staging + variables: + - name: REGION + value: eu-west-1 + secrets: + - name: DEPLOY_TOKEN + value: $STAGING_DEPLOY_TOKEN +env: + STAGING_DEPLOY_TOKEN: sealed-staging-plaintext-1a2b +inputs: + mode: apply +live_state: + environments: + staging: + name: staging + protection_rules: [] +token_permissions: + environments: write +expect: + exit_code: 1 + result: failed + outcomes: + environments: failed + mutations: + - "PUT /repos/{repo}/environments/production" + never: + - "POST /repos/{repo}/environments/staging/variables" + - "PUT /repos/{repo}/environments/staging/secrets" + stdout_contains: + - '"id" is not a permitted key' + - "::warning::environments: 2 operations after this failure were not attempted; re-run once it is fixed" + - 'environments: not attempted - creating variable "REGION" in environment "staging"' + - 'environments: not attempted - writing secret "DEPLOY_TOKEN" in environment "staging"' + summary_contains: + - 'not attempted - creating variable "REGION" in environment "staging"' + - 'not attempted - writing secret "DEPLOY_TOKEN" in environment "staging"' + leaks_nowhere: + - sealed-staging-plaintext-1a2b