From fa5121426dce25717acc8c6ce04341a787b98e18 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Wed, 11 Nov 2026 23:17:12 -0500 Subject: [PATCH 1/2] fix(environments): name a key GitHub only reports or does not echo on an entry or a nested entry An environment entry, and each entry of its nested lists, is an open shape: a key the shape does not declare passes the parse and rides the request. Two classes of such a key had no line naming them. A key GitHub only reports (the GET returns it, the write does not take it) is now named by a note in every run, from the declaration alone: id, node_id, url, html_url, created_at, updated_at and protection_rules on an entry, id and node_id on a deployment branch-policy pattern, created_at and updated_at on a variable. The three lists are pinned both ways to @octokit/openapi-types. The variable note lives in the shared variables engine, so actions_variables and agents_variables print it too. An undeclared key on a branch-policy pattern that the live pattern does not echo (a typo such as typ for type) is now named by a note in every run where the pattern exists. The planner sends such a key on the create call alone and compares it in no run, so it adds no drift; phantomNote takes an optional tail for a write the comparison does not gate, and every existing caller prints the sentence it printed before. No file that parsed before is refused. Each open shape that still accepts a reported key carries a COMPAT(v4) marker naming the parse refusal that replaces the note. --- docs/reference/coverage.md | 3 + docs/sections/environments.docs.yml | 13 +++ src/engine/diff.ts | 11 +- src/sections/environments/branch-policies.ts | 47 +++++++- src/sections/environments/index.ts | 28 ++++- src/sections/environments/reported-keys.ts | 66 +++++++++++ src/sections/environments/schema.ts | 2 + src/sections/shared/schema-helpers.ts | 1 + .../secrets-and-variables/variables-engine.ts | 44 +++++++- .../actions_variables.test.ts | 20 ++++ .../environments/environments.test.ts | 105 ++++++++++++++++++ ...ment-branch-policies-unknown-key-noted.yml | 46 ++++++++ 12 files changed, 373 insertions(+), 13 deletions(-) create mode 100644 src/sections/environments/reported-keys.ts create mode 100644 test/sections/environments/scenarios/environment-branch-policies-unknown-key-noted.yml diff --git a/docs/reference/coverage.md b/docs/reference/coverage.md index b9148f1e..5afef03e 100644 --- a/docs/reference/coverage.md +++ b/docs/reference/coverage.md @@ -225,6 +225,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. @@ -234,6 +235,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`) @@ -246,6 +248,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..1c4930c1 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"; @@ -20,6 +20,7 @@ import { } from "./endpoints.js"; import type { LiveEnvironmentBody } from "./index.js"; import type { NestedPlan } from "./nested.js"; +import { BRANCH_POLICY_REPORTED_KEYS, reportedKeysNote } from "./reported-keys.js"; import type { DeploymentBranchPolicyConfig } from "./schema.js"; /** "delete" like the nested variables list: patterns are readable, recreatable configuration. */ @@ -41,6 +42,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 +139,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 +185,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: readonly string[] = BRANCH_POLICY_REPORTED_KEYS.filter((key) => + Object.hasOwn(extra, key), + ); + const notes = + reported.length > 0 + ? [reportedKeysNote(label, reported, "POST", `so ${KEY_OUTCOME.join(" ")}`)] + : []; + if (existing === undefined) { + return notes; + } + const unechoed = phantomKeys(extra, existing).filter((key) => !reported.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 +248,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 +287,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..b793068f 100644 --- a/src/sections/environments/index.ts +++ b/src/sections/environments/index.ts @@ -38,6 +38,7 @@ import { planPinned, snapshotPins, } from "./pins.js"; +import { ENVIRONMENT_REPORTED_KEYS, reportedKeysNote } from "./reported-keys.js"; import { EnvironmentConfig, EnvironmentsConfig, environmentKey } from "./schema.js"; import { sharedSecretNotes, snapshotNested, withPins } from "./snapshot.js"; @@ -169,16 +170,30 @@ export function flattenEnvironment(live: LiveEnvironmentBody): Record, live: LiveEnvironmentBody | undefined, ): { drift: string[]; omitted: string[]; notes: string[] } { + const reported: readonly string[] = ENVIRONMENT_REPORTED_KEYS.filter((key) => + Object.hasOwn(settings, key), + ); + const notes = + reported.length > 0 + ? [ + reportedKeysNote( + label, + reported, + "PUT", + "so GitHub answers this environment's PUT with a 422 whenever apply sends one", + ), + ] + : []; if (live === undefined) { - return { drift: [missingDrift(label)], omitted: [], notes: [] }; + return { drift: [missingDrift(label)], omitted: [], notes }; } const flattened = flattenEnvironment(live); @@ -187,10 +202,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.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..22e2c16e --- /dev/null +++ b/src/sections/environments/reported-keys.ts @@ -0,0 +1,66 @@ +/** + * 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 from + * the declaration alone. 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 { agree } from "../../text.js"; +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 +>; + +/** `write` is the call that does not take the keys, `outcome` what follows from that. */ +export function reportedKeysNote( + label: string, + keys: readonly string[], + write: string, + outcome: string, +): string { + const list = keys.map((key) => `"${key}"`).join(", "); + const count = keys.length; + const them = agree(count, "it", "them"); + return ( + `${label}: ${list} ${agree(count, "is a value", "are values")} GitHub reports, not ${agree(count, "a setting", "settings")} it accepts ` + + `(the GET returns ${them}, the ${write} does not take ${them}), ${outcome}; remove ${them} from the settings file` + ); +} diff --git a/src/sections/environments/schema.ts b/src/sections/environments/schema.ts index 8c0e0e6a..6cd330af 100644 --- a/src/sections/environments/schema.ts +++ b/src/sections/environments/schema.ts @@ -43,6 +43,7 @@ const MAX_REVIEWERS = 6; /** GitHub's cap on pinned environments per repository. */ export const MAX_PINNED_ENVIRONMENTS = 10; +// 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. /** A branch pattern entry; an absent type is GitHub's own default, "branch". */ export const DeploymentBranchPolicyConfig = open({ name: z.string(), @@ -102,6 +103,7 @@ const DeploymentBranchPolicy = open(DeploymentBranchPolicyFlags.unwrap().shape) ), }); +// 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. /** One environment entry: the PUT body's keys, plus the routed scalar and the nested lists nested.ts strips before the PUT. */ export const EnvironmentConfig = open({ name: z.string(), diff --git a/src/sections/shared/schema-helpers.ts b/src/sections/shared/schema-helpers.ts index 32364e69..413fdafd 100644 --- a/src/sections/shared/schema-helpers.ts +++ b/src/sections/shared/schema-helpers.ts @@ -351,6 +351,7 @@ export function sealedSecretConfig(id: string) { return open({ name: secretName, value: z.string() }).meta({ id }); } +// 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. /** A repository-scope plain-text variable entry; the environments section's nested list is the same shape. */ 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..ae34312c 100644 --- a/src/sections/shared/secrets-and-variables/variables-engine.ts +++ b/src/sections/shared/secrets-and-variables/variables-engine.ts @@ -4,9 +4,12 @@ * (../../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 { agree } from "../../../text.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"; @@ -25,6 +28,41 @@ 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 +>; + +/** One variable's declared keys GitHub only reports, and the note naming them, read off the declaration alone. */ +function reportedKeys( + label: string, + extraKeys: object, +): { keys: readonly string[]; notes: string[] } { + const keys: readonly string[] = REPORTED_KEYS.filter((key) => Object.hasOwn(extraKeys, 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}, the PATCH does not take ${them}), 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 ${them} from the settings file`; + return { keys, notes: [note] }; +} + /** 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 +136,8 @@ export function planVariables< const label = `${scope.label}[${variable.name}]`; const existing = liveOf(variable); const { name: _name, value: _value, ...extraKeys } = variable; + const reported = reportedKeys(label, extraKeys); + notes.push(...reported.notes); if (!existing) { ops.push( scope.create({ @@ -127,7 +167,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..556b5a66 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, the PATCH does not take 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..914e62c0 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", {}], + ["the live id equals the declared one", { [ENV]: liveEnv("prod", { id: 1 }) }], + ["the live id differs", { [ENV]: liveEnv("prod", { id: 99 }) }], + ["the live body carries no id", { [ENV]: liveEnv("prod") }], + ] as Array<[string, Record]>)( + "an entry's id, which GitHub only reports, is named once when %s", + async (_state, routes) => { + // 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: 1 } 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, the PATCH does not take 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" From 110c68f19277d0c706be7ce49f4a267ac47a517f Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Thu, 12 Nov 2026 00:09:06 -0500 Subject: [PATCH 2/2] fix(environments): say the reported-key note once, name both variable writes, and pin the unattempted report The note for a key GitHub only reports was written twice, in the environments section and in the shared variables engine. It is now one function in src/sections/shared/reported-key-note.ts, called by the entry, the branch-policy pattern, and the variable. A variable that does not exist yet is written by a POST, so the variable note now says neither the POST nor the PATCH takes the key. The environmentDrift comment is two short paragraphs, and each COMPAT(v4) marker sits in its declaration's doc comment. A reported key declared null still draws its note, and a test row pins it: such a declaration drifts against the live value and rides the write. A new scenario pins what a refused entry leaves unattempted: the first environment's PUT answers 422, and the run names the second environment's variable and secret as not attempted, on stdout and in the summary. --- src/sections/environments/branch-policies.ts | 19 +++---- src/sections/environments/index.ts | 39 ++++++--------- src/sections/environments/reported-keys.ts | 22 ++------ src/sections/environments/schema.ts | 12 +++-- src/sections/shared/reported-key-note.ts | 38 ++++++++++++++ src/sections/shared/schema-helpers.ts | 6 ++- .../secrets-and-variables/variables-engine.ts | 29 ++++------- .../actions_variables.test.ts | 2 +- .../environments/environments.test.ts | 20 ++++---- ...pply-refused-entry-reports-unattempted.yml | 50 +++++++++++++++++++ 10 files changed, 149 insertions(+), 88 deletions(-) create mode 100644 src/sections/shared/reported-key-note.ts create mode 100644 test/sections/environments/scenarios/environments-apply-refused-entry-reports-unattempted.yml diff --git a/src/sections/environments/branch-policies.ts b/src/sections/environments/branch-policies.ts index 1c4930c1..2e6ef09b 100644 --- a/src/sections/environments/branch-policies.ts +++ b/src/sections/environments/branch-policies.ts @@ -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,7 +21,7 @@ import { } from "./endpoints.js"; import type { LiveEnvironmentBody } from "./index.js"; import type { NestedPlan } from "./nested.js"; -import { BRANCH_POLICY_REPORTED_KEYS, reportedKeysNote } from "./reported-keys.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. */ @@ -200,17 +201,17 @@ function undeclaredKeyNotes( ): string[] { const label = patternLabel(envName, pattern.name); const { name: _name, type: _type, ...extra } = pattern as Record; - const reported: readonly string[] = BRANCH_POLICY_REPORTED_KEYS.filter((key) => - Object.hasOwn(extra, key), - ); - const notes = - reported.length > 0 - ? [reportedKeysNote(label, reported, "POST", `so ${KEY_OUTCOME.join(" ")}`)] - : []; + 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.includes(key)); + + 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)); } diff --git a/src/sections/environments/index.ts b/src/sections/environments/index.ts index b793068f..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,7 +39,7 @@ import { planPinned, snapshotPins, } from "./pins.js"; -import { ENVIRONMENT_REPORTED_KEYS, reportedKeysNote } from "./reported-keys.js"; +import { ENVIRONMENT_REPORTED_KEYS } from "./reported-keys.js"; import { EnvironmentConfig, EnvironmentsConfig, environmentKey } from "./schema.js"; import { sharedSecretNotes, snapshotNested, withPins } from "./snapshot.js"; @@ -165,33 +166,25 @@ export function flattenEnvironment(live: LiveEnvironmentBody): Record, live: LiveEnvironmentBody | undefined, ): { drift: string[]; omitted: string[]; notes: string[] } { - const reported: readonly string[] = ENVIRONMENT_REPORTED_KEYS.filter((key) => - Object.hasOwn(settings, key), - ); - const notes = - reported.length > 0 - ? [ - reportedKeysNote( - label, - reported, - "PUT", - "so GitHub answers this environment's PUT with a 422 whenever apply sends one", - ), - ] - : []; + 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 }; } @@ -202,7 +195,7 @@ function environmentDrift( sweep: replaceSweep(EnvironmentConfig), }).map((delta) => renderDelta(label, delta)); const phantom = phantomKeys(settings, flattened).filter( - (key) => !Object.hasOwn(EnvironmentConfig.shape, key) && !reported.includes(key), + (key) => !Object.hasOwn(EnvironmentConfig.shape, key) && !reported.keys.includes(key), ); if (phantom.length > 0) { notes.push(phantomNote(label, phantom, "environment", "this PUT will re-run")); diff --git a/src/sections/environments/reported-keys.ts b/src/sections/environments/reported-keys.ts index 22e2c16e..b4fe631e 100644 --- a/src/sections/environments/reported-keys.ts +++ b/src/sections/environments/reported-keys.ts @@ -1,11 +1,11 @@ /** * 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 from - * the declaration alone. Each list is pinned both ways to the vendored spec, by its satisfies and its MustBeNever. + * 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 { agree } from "../../text.js"; import type { MustBeNever } from "../../types.js"; type EnvironmentPutBody = NonNullable< @@ -48,19 +48,3 @@ export const BRANCH_POLICY_REPORTED_KEYS = [ type _BranchPolicyKeysComplete = MustBeNever< Exclude >; - -/** `write` is the call that does not take the keys, `outcome` what follows from that. */ -export function reportedKeysNote( - label: string, - keys: readonly string[], - write: string, - outcome: string, -): string { - const list = keys.map((key) => `"${key}"`).join(", "); - const count = keys.length; - const them = agree(count, "it", "them"); - return ( - `${label}: ${list} ${agree(count, "is a value", "are values")} GitHub reports, not ${agree(count, "a setting", "settings")} it accepts ` + - `(the GET returns ${them}, the ${write} does not take ${them}), ${outcome}; remove ${them} from the settings file` - ); -} diff --git a/src/sections/environments/schema.ts b/src/sections/environments/schema.ts index 6cd330af..64448203 100644 --- a/src/sections/environments/schema.ts +++ b/src/sections/environments/schema.ts @@ -43,8 +43,10 @@ const MAX_REVIEWERS = 6; /** GitHub's cap on pinned environments per repository. */ export const MAX_PINNED_ENVIRONMENTS = 10; -// 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. -/** 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(), @@ -103,8 +105,10 @@ const DeploymentBranchPolicy = open(DeploymentBranchPolicyFlags.unwrap().shape) ), }); -// 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. -/** 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 413fdafd..82cf61d6 100644 --- a/src/sections/shared/schema-helpers.ts +++ b/src/sections/shared/schema-helpers.ts @@ -351,8 +351,10 @@ export function sealedSecretConfig(id: string) { return open({ name: secretName, value: z.string() }).meta({ id }); } -// 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. -/** 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 ae34312c..dfb59d30 100644 --- a/src/sections/shared/secrets-and-variables/variables-engine.ts +++ b/src/sections/shared/secrets-and-variables/variables-engine.ts @@ -8,12 +8,12 @@ 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 { agree } from "../../../text.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, @@ -44,24 +44,13 @@ type _ReportedKeysComplete = MustBeNever< Exclude >; -/** One variable's declared keys GitHub only reports, and the note naming them, read off the declaration alone. */ -function reportedKeys( - label: string, - extraKeys: object, -): { keys: readonly string[]; notes: string[] } { - const keys: readonly string[] = REPORTED_KEYS.filter((key) => Object.hasOwn(extraKeys, 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}, the PATCH does not take ${them}), 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 ${them} from the settings file`; - return { keys, notes: [note] }; -} +/** 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 { @@ -136,7 +125,7 @@ export function planVariables< const label = `${scope.label}[${variable.name}]`; const existing = liveOf(variable); const { name: _name, value: _value, ...extraKeys } = variable; - const reported = reportedKeys(label, extraKeys); + const reported = reportedKeyNote(label, extraKeys, REPORTED); notes.push(...reported.notes); if (!existing) { ops.push( diff --git a/test/sections/actions_variables/actions_variables.test.ts b/test/sections/actions_variables/actions_variables.test.ts index 556b5a66..44124f5c 100644 --- a/test/sections/actions_variables/actions_variables.test.ts +++ b/test/sections/actions_variables/actions_variables.test.ts @@ -294,7 +294,7 @@ describe("actions_variables", () => { 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, the PATCH does not take 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", + "(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 914e62c0..e3cb2e0b 100644 --- a/test/sections/environments/environments.test.ts +++ b/test/sections/environments/environments.test.ts @@ -699,17 +699,17 @@ describe("environments keys outside the entry shapes", () => { const ENV = "GET /repos/o/r/environments/prod"; test.each([ - ["the environment is missing", {}], - ["the live id equals the declared one", { [ENV]: liveEnv("prod", { id: 1 }) }], - ["the live id differs", { [ENV]: liveEnv("prod", { id: 99 }) }], - ["the live body carries no id", { [ENV]: liveEnv("prod") }], - ] as Array<[string, Record]>)( + ["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) => { + 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: 1 } as EnvironmentConfig, - ]); + 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', ]); @@ -757,7 +757,7 @@ describe("environments keys outside the entry shapes", () => { ]); expect(checked.notes).toEqual([ 'environments[prod].variables[X]: "updated_at" is a value GitHub reports, not a setting it accepts ' + - "(the GET returns it, the PATCH does not take 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", + "(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/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