fix: update access commands (list, update, add) to Utilize Permission Chain Directly. - #3856
Open
hiimtaylorjones wants to merge 2 commits into
Open
fix: update access commands (list, update, add) to Utilize Permission Chain Directly.#3856hiimtaylorjones wants to merge 2 commits into
access commands (list, update, add) to Utilize Permission Chain Directly.#3856hiimtaylorjones wants to merge 2 commits into
Conversation
`heroku access` rendered a permissions column for some team apps but not others because the CLI re-derived "is this a team app?" from the owner email string (`/@herokumanager\.com$/`) instead of reading what the Platform API actually sent. The API only serializes each collaborator's `permissions` array for genuine team apps (App#team?). The CLI's email regex diverges from that in two cases: 1. Team service accounts — `foo+service@herokumanager.com` matches the CLI regex (column shown) but the API's `team_email?` excludes `+service@` (no permissions sent). The CLI's own row filter already rejects `herokumanager.com` collaborators, so it was internally inconsistent too. 2. Missing/null owner in the payload — `app.owner?.email` → undefined → column hidden, even when the API included permission data for a real team app. Drive `showPermissions` off the presence of `permissions` in the collaborators response — the authoritative source — so the column tracks exactly what the API sent. `buildTableColumns` and the per-row mapping already gate on `showPermissions`, so no other changes are needed. `printAccess` no longer needs the `app` argument. Adds unit tests covering a team service-account-owned app and a team app with a null/absent owner, alongside the existing personal- and team-app cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 6, 2026 23:48 — with 
GitHub Actions
Inactive
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 6, 2026 23:48 — with
GitHub Actions
Inactive
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 6, 2026 23:48 — with
GitHub Actions
Inactive
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 6, 2026 23:48 — with
GitHub Actions
Inactive
hiimtaylorjones
marked this pull request as ready for review
August 7, 2026 00:29
…ands Extends the previous access-list fix to `access:add`, `access:update`, and the members lookup in `access` (index). All three re-derived "is this a team app?" from the owner email regex (`isTeamApp`), which diverges from the Platform API's own definition (`App#team?`, i.e. `!!team`) in the same two cases: - `access:update` refused to update a genuine team app when `owner` was absent from the payload (`isTeamApp(undefined)` → false → hard error), even though the API would accept the PATCH. - `access:add` skipped the team-features lookup for a null-owner team app, silently dropping `--permissions`; and for other herokumanager.com-shaped owners it scraped the team name from the email string rather than using the real team name. Key detection off the authoritative `team` field the API sends, and use `team.name` for the `/teams/<name>/features` and `/teams/<name>/members` lookups (falling back to the email-derived name only if `team.name` is absent). This makes the CLI agree with the API by construction and removes the now-unused `isTeamApp` imports from these commands. `isTeamApp` remains in team-utils for `apps:transfer`. Also makes the `teamApp()` test stub faithful to a real team-app payload (it now includes a `team` object) and adds `teamAppWithoutOwner()` to cover the null-owner case in the add and update suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 7, 2026 00:59 — with
GitHub Actions
Inactive
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 7, 2026 00:59 — with
GitHub Actions
Inactive
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 7, 2026 00:59 — with
GitHub Actions
Inactive
hiimtaylorjones
temporarily deployed
to
AcceptanceTests
August 7, 2026 00:59 — with
GitHub Actions
Inactive
access commands (list, update, add) to Utilize Permission Chain Directly.
access commands (list, update, add) to Utilize Permission Chain Directly.access commands (list, update, add) to Utilize Permission Chain Directly.
access commands (list, update, add) to Utilize Permission Chain Directly.access commands (list, update, add) to Utilize Permission Chain Directly.
access commands (list, update, add) to Utilize Permission Chain Directly.access commands (list, update, add) to Utilize Permission Chain Directly.
michaelmalave
approved these changes
Aug 7, 2026
michaelmalave
left a comment
Contributor
There was a problem hiding this comment.
Nice update. LGTM!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's Changing
When listing the permissions that an app directly has, I want to switch out the
isTeamAppcheck to directly tap into the collaborators permission array. This change will allow us to leverage some future API updates around more granular permission controls for apps.I've also made similar changes to the
access:addandaccess:updatecommands. They also were utilizing this team check.Tests Suites Changes
Adds unit tests for the two previously-broken cases alongside the existing ones:
All
test/unit/commands/access/*.unit.test.tspass (13 passing).Verification
Ran this against a scenario I'd been working with in staging. Details will be provided in thread outside of this.