docs(mint): add the App permission rollout runbook and the contributor rule - #6955
Open
waynesun09 wants to merge 3 commits into
Open
docs(mint): add the App permission rollout runbook and the contributor rule#6955waynesun09 wants to merge 3 commits into
waynesun09 wants to merge 3 commits into
Conversation
…r rule Adding a permission to a mint role is an operational rollout, not a deploy: GitHub rejects the whole installation-token request when any requested permission is ungranted, and existing installations grant nothing until an org owner Accepts the App update. The v0.40.0 packages:read rollout exercised the full flow — contributor, App owner, mint admin, installation owners, CLI users — so write it down as the standard procedure. - New guide docs/guides/infrastructure/app-permission-rollout.md: roles at a glance, the five rules that explain the ordering, one step-by-step section per persona with exact UI paths, commands and the log lines to watch, a state -> log -> token -> setup-output table, how to finish the rollout, and troubleshooting for the failure modes seen in practice. - docs/contributing/mintcore.md: "Adding a permission to a role" — the declaration sites that change together, the parity tests, the backward-compatibility rule (new permissions go in optionalRolePermissions for the rollout window), and the BREAKING CHANGE commit convention. - infrastructure-reference.md: the rollout section becomes a short mechanism paragraph plus a link; operator steps and test-pool specifics move out of the reference page. - Sidebar entry, and pointers from mint-administration and operations. Refs #6649, #6945. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
PR Summary by QodoDocument GitHub App permission rollout and contributor safeguards
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
|
🤖 Review · Commit: |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+21
to
+29
| Five roles are involved, usually five different people: | ||
|
|
||
| | Role | What they do | | ||
| |------|--------------| | ||
| | **Contributor** | Lands one PR that adds the permission to the mint's role map, the embedded copy of the mint, and the App manifest, and marks it optional for the duration of the rollout. | | ||
| | **App owner** | Adds the permission on the GitHub App registration, which puts a pending update in front of every installation. | | ||
| | **Mint admin** | Deploys the mint that understands the new map, confirms the deploy landed, and watches the logs for installations that have not accepted. | | ||
| | **Installation owner** | An **owner** of an organization the App is installed on. Accepts the pending update for that organization. | | ||
| | **CLI user / repo admin** | Upgrades the fullsend CLI and re-runs `fullsend github setup <owner/repo>` for their repository. | |
There was a problem hiding this comment.
1. Runbook mixes multiple audiences 📜 Skill insight ⌂ Architecture
The new guide explicitly assigns procedures to five personas—Contributor, App owner, Mint admin, Installation owner, and CLI user—rather than targeting a single audience. This violates the requirement that each guide address only one audience.
Agent Prompt
## Issue description
The new guide combines procedures for contributors, App owners, mint administrators, installation owners, and CLI users, violating the single-audience guide requirement.
## Issue Context
Keep each guide focused on one audience and place cross-audience procedures in separate guides or link to audience-specific documents.
## Fix Focus Areas
- docs/guides/infrastructure/app-permission-rollout.md[21-29]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+69
to
+73
| ## Step by step | ||
|
|
||
| The subsections below are in rollout order. Each one is a different person's job. | ||
|
|
||
| ### Contributor: land the change |
There was a problem hiding this comment.
2. Runbook lacks prerequisites 📜 Skill insight ✧ Quality
The new guide begins with background and rollout rules but has no clearly labeled Prerequisites section before its procedural steps. Readers therefore lack a stated list of required access, tools, and permissions before starting the procedure.
Agent Prompt
## Issue description
The guide has procedural sections but no clearly labeled prerequisites section before the first procedure.
## Issue Context
Document the required GitHub, mint, GCP, CLI, and organization-owner access before the step-by-step content.
## Fix Focus Areas
- docs/guides/infrastructure/app-permission-rollout.md[1-19]
- docs/guides/infrastructure/app-permission-rollout.md[69-73]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+34
to
+46
| ## The rules | ||
|
|
||
| These five facts explain every step below. If a step ever looks optional, come back here. | ||
|
|
||
| 1. **There is no partial downscope.** When the mint POSTs for an installation token, GitHub rejects | ||
| the *entire* request with `422` if any requested permission is not granted on that installation. | ||
| It does not quietly return a smaller token. A role map that asks for one ungranted permission | ||
| therefore breaks every token for that role, not just the new capability. | ||
| 2. **The installation object lists accepted permissions only.** The mint looks the installation up | ||
| (`GET /orgs/<org>/installation`, `GET /repos/<owner>/<repo>/installation`) and reads the | ||
| `permissions` map. A pending update never appears there — only what the organization has already | ||
| accepted. This is why the App registration can be updated before the mint is redeployed without | ||
| breaking anything, and why the mint can decide what to request *before* it POSTs. |
There was a problem hiding this comment.
4. Runbook inlines architecture details 📜 Skill insight ⚙ Maintainability
The guide contains an extensive inline explanation of mint token negotiation, GitHub installation permission state, optional downscoping, and rollout ordering instead of linking architectural context to a normative reference. This duplicates architectural behavior in a procedural guide and can become stale.
Agent Prompt
## Issue description
The guide restates detailed architecture and mechanism semantics inline, including token rejection behavior and installation permission state.
## Issue Context
Retain concise operational guidance, but link the architectural mechanism to the authoritative reference or architecture documentation.
## Fix Focus Areas
- docs/guides/infrastructure/app-permission-rollout.md[34-67]
- docs/guides/infrastructure/infrastructure-reference.md[125-146]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+86
to
+89
| ### App owner: update the App registration | ||
|
|
||
| Do this once per App set you own. For the hosted apps the coder App slug is `fullsend-ai-coder`; for | ||
| your own app set it is `<app-set>-coder`. |
There was a problem hiding this comment.
5. Hardcoded hosted app identifiers 📘 Rule violation ⛨ Security
The new runbook embeds the organization-specific hosted App slug fullsend-ai-coder and references the fullsend-ai organization as operational examples. These environment-specific identifiers should be represented with placeholders or loaded from configuration rather than hardcoded in documentation.
Agent Prompt
## Issue description
The runbook hardcodes the hosted App slug `fullsend-ai-coder`, an organization-specific deployment identifier.
## Issue Context
Use placeholders such as `<app-slug>` and `<owner-org>` throughout the generic runbook, or clearly label the value as a non-sensitive illustrative example if it is intentionally public.
## Fix Focus Areas
- docs/guides/infrastructure/app-permission-rollout.md[86-89]
- docs/guides/infrastructure/infrastructure-reference.md[143-146]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Site previewPreview: https://470cd226-site.fullsend-ai.workers.dev Commit: |
…unbook
Review follow-ups on the rollout runbook: show the HTTP 422 body
(`{"error": …}`, message only) separately from the mint log line that
carries the `failed to mint token: org=…` prefix; scope the
"nobody blocks anybody" claim to acceptance and CLI upgrade, since the
first three steps are ordered; make the CLI step explicitly optional for
the permission itself; note that an empty permissions map is treated like
a missing one; mention the Organization permissions section, personal
account installations, Cloudflare Worker logs, and the --openai-* flags;
promote the operations note to its own section; add the guide to the
guides index.
Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
State the access and tooling each persona needs before their step, and tell readers to jump to their own section. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
|
🤖 Review · Commit: |
|
🤖 Finished Review · ✅ Success · Started 11:27 AM UTC · Completed 11:45 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $8.55 |
|
Risk Assessment: moderate (2/5) DetailsDocs-only PR adding a rollout runbook and contributor rule (467 lines, 7 files) with no protected paths, security-sensitive content, CI, or dependency changes; moderate score reflects the non-trivial size and active churn in the surrounding docs area, offset by the inherently low-risk nature of additive documentation. |
ReviewFindingsLow
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| | `FULLSEND_GCP_PROJECT_ID` | Repo secret | GCP project ID where Agent Platform is enabled | `my-gcp-project` | | ||
| | `FULLSEND_GCP_WIF_PROVIDER` | Repo secret | Full WIF provider resource name for OIDC authentication | `projects/123456789/locations/global/...` | | ||
|
|
||
|
|
There was a problem hiding this comment.
[low] spurious blank line
The diff adds a stray blank line between the GitHub table and the ### GitLab heading (two blank lines instead of one). The rest of the file uses exactly one blank line between a table and a heading.
Suggested fix: Remove the extra blank line so there is a single blank line between the table and ### GitLab, matching the rest of the file.
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
1 participant
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.
Summary
Write down the standard procedure for rolling out a new GitHub App permission to a mint role, as exercised end to end for
packages:readin v0.40.0.docs/guides/infrastructure/app-permission-rollout.md— walkthrough form: roles at a glance (contributor · App owner · mint admin · installation owner · CLI user), the five rules that explain the fixed order (code → App registration → mint deploy → CLI release → outreach), one step-by-step section per persona with exact GitHub UI paths, commands, and the three mint log lines to watch, a state → log → token →github setupoutput table, how to finish the rollout (remove the optional entry), and troubleshooting for the failure modes met in practice.docs/contributing/mintcore.md— "Adding a permission to a role": the three declaration sites that change together (canonicalRolePermissions, the GCF embed,AgentAppConfig), the parity tests, the backward-compatibility requirement that a permission existing installations have not granted goes intooptionalRolePermissionsin the same PR, that the entry is temporary, and the!/BREAKING CHANGEcommit convention.infrastructure-reference.md— the rollout section is reduced to a mechanism paragraph plus a link; operator steps and test-pool specifics no longer live on the reference page.mint-administration.md(add-role area) andoperations.md(a new "Accepting a GitHub App permission update" note for org admins).Why
The rollout mechanism landed in #6654 but the operator procedure was folded into the reference page and mixed with test-infrastructure details. Operators need a page they can follow without having been in the room; contributors need the rule that keeps the next permission change backward compatible.
Verification
pre-commiton all changed files: docs link scope, markdown links, whitespace, prettier — all pass.npx vitepress build docs— build complete; theoperations.mdanchor link into the runbook resolves in the built HTML.<org>,<app-slug>,<project>,<mint-url>) throughout.Refs #6649, #6945.