Skip to content

graformer: name organisation resources in the plan summary - #74

Merged
dev-milos merged 3 commits into
mainfrom
fix/plan-summary-org-resources
Aug 14, 2026
Merged

graformer: name organisation resources in the plan summary#74
dev-milos merged 3 commits into
mainfrom
fix/plan-summary-org-resources

Conversation

@dev-milos

@dev-milos dev-milos commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Closes G-Research/gr-oss#1433

The plan summary 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:

unknown type github_membership. resource address: github_membership.member["<user>"]
unknown type github_team. resource address: github_team.team["<team>"]
unknown type github_team_membership. resource address: github_team_membership.membership["<user>/<team>"]

After:

github_membership :: <user> (admin)
github_team :: <team>
github_team_membership :: <user>/<team> (maintainer)

Why it matters

This 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.

Everything else in the summary was given readable formatting, including the deployment policy
types added most recently. These three were missed.

Two decisions worth a look

The role is included for github_membership. Promotion and demotion between admin and
member are otherwise invisible in the summary — the line would be identical either way. Since
the role is a value rather than an identity, both sides of the change are read and a change is
rendered as a transition:

github_membership :: <user> (member -> admin)
github_team :: <old-name> -> <new-name>

Callers pass change.before for updates, which is harmless for the stable identity fields
every other type renders, but would have shown the role being replaced rather than the one
being applied.

Team membership takes its name from the resource address, not teamMap. teamMap is
built from planJson.data, so it covers only data.github_team lookups; organisation teams
are resources and are absent from it, which would render undefined. The address already
carries <username>/<team> in exactly the form wanted. The pattern is anchored on the resource
type, the way the branch protection one already is — unanchored it would take the first
bracketed key, which is correct while these are root-level but would silently return a module
key if they ever moved, without the fallback firing. There is a comment explaining the choice
so it does not get "corrected" to teamMap later.

Verification

The function was extracted from the action and exercised against the real address shapes
observed during release testing:

Input Output
github_team.team["<team>"] github_team :: <team>
github_membership.member["<user>"], role admin github_membership :: <user> (admin)
same, role member github_membership :: <user> (member)
github_team_membership.membership["<user>/<team>"] github_team_membership :: <user>/<team> (maintainer)
an existing type unchanged
a genuinely unknown type still falls through as before

No other branch is touched.

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
"<username>/<team>". teamMap covers only data.github_team lookups, so organisation
teams are absent from it and would render as undefined.
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.
@dev-milos
dev-milos marked this pull request as ready for review August 14, 2026 11:05

@bitwisecook bitwisecook left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dev-milos
dev-milos merged commit 59a1c2c into main Aug 14, 2026
1 check passed
@dev-milos
dev-milos deleted the fix/plan-summary-org-resources branch August 14, 2026 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants