From 10f9029b201c0455f55693fac4275ba97acaeaee Mon Sep 17 00:00:00 2001 From: dev-milos Date: Fri, 14 Aug 2026 12:40:53 +0200 Subject: [PATCH 1/3] graformer: name organisation resources in the plan summary The formatter knew nine repository-level resource types and none of the three the organisation feature introduced, so every team, membership and team membership fell through to the unknown-type branch. That is the output a reviewer reads when approving a change to who belongs to the organisation. The bootstrap import rendered entirely as unknown-type lines, and so does the case that matters more: a plan removing a person shows an address rather than a statement of who is being removed. The membership role is included because promotion and demotion between admin and member are otherwise invisible in the summary. Team membership takes its name from the resource address, which already carries "/". teamMap covers only data.github_team lookups, so organisation teams are absent from it and would render as undefined. --- .github/actions/graformer/action.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/actions/graformer/action.yaml b/.github/actions/graformer/action.yaml index 437d561..984d9dd 100644 --- a/.github/actions/graformer/action.yaml +++ b/.github/actions/graformer/action.yaml @@ -117,6 +117,14 @@ runs: return `${resourceChange.type} :: ${actualChange.repository}/${actualChange.environment} (${pattern})`; }else if(resourceChange.type === 'github_repository_custom_property'){ return `${resourceChange.type} :: ${actualChange.repository}/${actualChange.property_name}`; + }else if(resourceChange.type === 'github_team'){ + return `${resourceChange.type} :: ${actualChange.name}`; + }else if(resourceChange.type === 'github_membership'){ + return `${resourceChange.type} :: ${actualChange.username} (${actualChange.role})`; + }else if(resourceChange.type === 'github_team_membership'){ + const match = resourceChange.address.match(/\["([^"]+)"\]/); + const membership = match ? match[1] : actualChange.username; + return `${resourceChange.type} :: ${membership} (${actualChange.role})`; } else { return `unknown type ${resourceChange.type}. resource address: ${resourceChange.address}` } From 0f0d7b70da22330e9c1bf76da84a4b0d8d142d30 Mon Sep 17 00:00:00 2001 From: dev-milos Date: Fri, 14 Aug 2026 12:57:26 +0200 Subject: [PATCH 2/3] graformer: read both sides of a change, and anchor the membership key Two defects in the organisation formatting. Updates are passed change.before, so the role rendered for a promotion was the one being replaced: member to admin printed (member), and admin to member printed (admin) - inverted, on the single line a reviewer uses to approve an organisation membership change. A team rename printed the old name for the same reason. This did not show up before because every previously handled type renders only stable identity fields, where before and after agree. Both sides are now read, and a change is shown as a transition. The membership key pattern was unanchored, so it took the first bracketed key in the address. That is correct while these resources are root-level, but if they ever moved into a keyed module it would silently return the module key, and the fallback would not fire because a match did occur. It is now anchored on the resource type, the way the branch protection pattern already is. --- .github/actions/graformer/action.yaml | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/.github/actions/graformer/action.yaml b/.github/actions/graformer/action.yaml index 984d9dd..61b6ed3 100644 --- a/.github/actions/graformer/action.yaml +++ b/.github/actions/graformer/action.yaml @@ -96,6 +96,17 @@ runs: recreate: [] }; + // Callers pass change.before for updates, which is fine for stable identity fields + // but wrong for anything that changes. Read both sides and show the transition. + function renderField(resourceChange, field){ + const from = resourceChange.change.before && resourceChange.change.before[field]; + const to = resourceChange.change.after && resourceChange.change.after[field]; + if (from && to && from !== to) { + return `${from} -> ${to}`; + } + return to || from; + } + function getResourceNameFromChange(resourceChange, actualChange){ if(resourceChange.type === 'github_repository'){ return `${resourceChange.type} :: ${actualChange.name}` @@ -118,13 +129,16 @@ runs: }else if(resourceChange.type === 'github_repository_custom_property'){ return `${resourceChange.type} :: ${actualChange.repository}/${actualChange.property_name}`; }else if(resourceChange.type === 'github_team'){ - return `${resourceChange.type} :: ${actualChange.name}`; + return `${resourceChange.type} :: ${renderField(resourceChange, 'name')}`; }else if(resourceChange.type === 'github_membership'){ - return `${resourceChange.type} :: ${actualChange.username} (${actualChange.role})`; + return `${resourceChange.type} :: ${renderField(resourceChange, 'username')} (${renderField(resourceChange, 'role')})`; }else if(resourceChange.type === 'github_team_membership'){ - const match = resourceChange.address.match(/\["([^"]+)"\]/); - const membership = match ? match[1] : actualChange.username; - return `${resourceChange.type} :: ${membership} (${actualChange.role})`; + // The address key is already "/"; the team is not in teamMap, + // which only covers data.github_team lookups. Anchored on the resource type so a + // module key can never be matched instead. + const match = resourceChange.address.match(/github_team_membership\.[^.[]+\["([^"]+)"\]$/); + const membership = match ? match[1] : renderField(resourceChange, 'username'); + return `${resourceChange.type} :: ${membership} (${renderField(resourceChange, 'role')})`; } else { return `unknown type ${resourceChange.type}. resource address: ${resourceChange.address}` } From deee8a57d1a64ad618c4563604e73ecb8f671151 Mon Sep 17 00:00:00 2001 From: dev-milos Date: Fri, 14 Aug 2026 13:04:54 +0200 Subject: [PATCH 3/3] drop the code comments; the rationale is in the pull request --- .github/actions/graformer/action.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/actions/graformer/action.yaml b/.github/actions/graformer/action.yaml index 61b6ed3..a9fb025 100644 --- a/.github/actions/graformer/action.yaml +++ b/.github/actions/graformer/action.yaml @@ -96,8 +96,6 @@ runs: recreate: [] }; - // Callers pass change.before for updates, which is fine for stable identity fields - // but wrong for anything that changes. Read both sides and show the transition. function renderField(resourceChange, field){ const from = resourceChange.change.before && resourceChange.change.before[field]; const to = resourceChange.change.after && resourceChange.change.after[field]; @@ -133,9 +131,6 @@ runs: }else if(resourceChange.type === 'github_membership'){ return `${resourceChange.type} :: ${renderField(resourceChange, 'username')} (${renderField(resourceChange, 'role')})`; }else if(resourceChange.type === 'github_team_membership'){ - // The address key is already "/"; the team is not in teamMap, - // which only covers data.github_team lookups. Anchored on the resource type so a - // module key can never be matched instead. const match = resourceChange.address.match(/github_team_membership\.[^.[]+\["([^"]+)"\]$/); const membership = match ? match[1] : renderField(resourceChange, 'username'); return `${resourceChange.type} :: ${membership} (${renderField(resourceChange, 'role')})`;