Skip to content

docs(#5311): add ADR 0073 for named mint privilege levels - #5328

Merged
ifireball merged 4 commits into
mainfrom
agent/5311-named-mint-privilege-levels
Jul 23, 2026
Merged

docs(#5311): add ADR 0073 for named mint privilege levels#5328
ifireball merged 4 commits into
mainfrom
agent/5311-named-mint-privilege-levels

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Add ADR 0073 deciding the mechanism for named privilege levels (read, write) under agent roles, covering mint API, custom role config, client support, acquisition semantics, and harness privilege_levels flag
  • Update docs/architecture.md with the decision under both Agent Identity Provider and Agent Harness sections

Related Issue

Closes #5311

Blocks: #2821 (human-gated permission adjustments ADR)
Implementation follow-on: #2823 (mint implementation)
Topic: #5312

Changes

ADR 0073 (docs/ADRs/0073-named-mint-privilege-levels.md)

New ADR covering seven required areas:

  1. Role levels — ordered named levels per role (read default, write as current max permissions)
  2. Mint API — optional level field in token requests (default read)
  3. Custom roles — backward-compatible CUSTOM_ROLE_PERMISSIONS extension with auto-detected multi-level JSON shape
  4. Clientsmintclient and CLI support for level selection
  5. Acquisition semantics — harness is the sole caller that selects privilege levels per phase
  6. Write level definitionwrite equals current max permissions; falls back to read when not defined
  7. Harness privilege_levels flag — maps run phases to levels with default key; backward-compatible default: write when omitted

docs/architecture.md

  • Added named privilege levels decision to Agent Identity Provider section
  • Added phase-scoped privilege levels decision to Agent Harness section

Testing

Checklist


Closes #5311

Post-script verification

  • Branch is not main/master (agent/5311-named-mint-privilege-levels)
  • Secret scan passed (gitleaks — 24d41836a2de6b7abaa6b20be2bdac9a61f7e0d3..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Introduce ADR 0073 deciding the mechanism for named privilege levels
under agent roles (read/write), covering mint API, custom role config,
client support, acquisition semantics, and harness privilege_levels
flag. Update docs/architecture.md with the decision.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Jul 19, 2026
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 19, 2026 17:44
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:45 PM UTC · Completed 5:59 PM UTC
Commit: eae5202 · View workflow run →

@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown

Site preview

Preview: https://ffd2c60a-site.fullsend-ai.workers.dev

Commit: 9141be038ef4dc5b34ae15e49fd1e7d35e4f7f1d

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ifireball ifireball self-assigned this Jul 19, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [internal-consistency] docs/ADRs/0073-named-mint-privilege-levels.md:44 — The ADR defines write as "the permission set currently granted by each built-in role" but does not specify how the read permission set is derived for built-in roles. Since the superset invariant requires read to be a subset of write, the derivation rule matters for implementation. Additionally, custom roles using the flat CUSTOM_ROLE_PERMISSIONS format have their permissions interpreted as the read level, meaning they would have no write level unless they use the multi-level format.

  • [edge-case-handling] docs/ADRs/0073-named-mint-privilege-levels.md:59 — The CUSTOM_ROLE_PERMISSIONS auto-detection mechanism does not explicitly state whether mixed formats (some roles flat, some multi-level) within a single JSON value are supported.

Comment thread docs/ADRs/0073-named-mint-privilege-levels.md
Comment thread docs/ADRs/0073-named-mint-privilege-levels.md
@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jul 19, 2026
@ifireball

Copy link
Copy Markdown
Member

/fs-fix the fullsend-ai-review comments according to the following:

The "read" permission levels for the built-in roles are determine by taking the current permissions sets and removing a the *:write permissions.

We will allow mixed format CUSTOM_ROLE_PERMISSIONS since we can write the detection code to allow it.

The following statement in the "consequences section is inaccurate:

The mint API remains backward-compatible; omitting level produces the same tokens as today.

The mint API itself will default to "read" level and therefore would return narrower tokens then what would be returned today

The same goes for the combination of the API and the clients

Only the use with harness files that omit privilege_levels remains compatible because of the default to write.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Fix · ❌ Terminated · Started 4:04 AM UTC · Ended 4:11 AM UTC
Commit: 3d48dce · View workflow run →

…d compatibility

Address review feedback on ADR 0073:
- Specify that built-in role read levels are derived by removing *:write
  permissions from the current permission set
- Clarify that CUSTOM_ROLE_PERMISSIONS supports mixed format within a
  single value (some roles flat, some multi-level), with per-role detection
- Fix inaccurate backward-compatibility statement: the mint API defaults to
  read (narrower tokens), not current behavior; only harness files omitting
  privilege_levels remain compatible via the write default

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all three review findings from human instruction on ADR 0073: (1) specified how built-in role read levels are derived (removing *:write permissions from current set), (2) clarified that CUSTOM_ROLE_PERMISSIONS supports mixed format within a single value with per-role detection, (3) corrected the inaccurate backward-compatibility statement — the mint API defaults to read (narrower tokens than today), and only harness-level privilege_levels omission preserves backward compatibility via the write default. Both docs/ADRs/0073-named-mint-privilege-levels.md and docs/architecture.md were updated consistently.

Fixed (3):

  1. *Built-in role read levels should be derived by removing :write permissions (docs/ADRs/0073-named-mint-privilege-levels.md): Added explicit statement in Role levels section that read level for each built-in role is derived by taking the role's current permission set and removing all *:write permissions. Updated architecture.md summary to match.
  2. Allow mixed format CUSTOM_ROLE_PERMISSIONS with per-role detection (docs/ADRs/0073-named-mint-privilege-levels.md): Changed 'auto-detects the format by checking' to 'auto-detects the format per role by checking'. Added paragraph stating mixed format is allowed within a single CUSTOM_ROLE_PERMISSIONS value. Updated architecture.md to mention mixed format support per role.
  3. Backward-compatibility statement in Consequences is inaccurate (docs/ADRs/0073-named-mint-privilege-levels.md): Replaced 'The mint API remains backward-compatible; omitting level produces the same tokens as today' with accurate statement: mint API defaults to read, producing narrower tokens. Clarified that only harness files omitting privilege_levels remain compatible because the harness defaults to write. Updated architecture.md to note omitting level produces narrower tokens.

Tests: passed

Updated by fullsend fix agent

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

E2E tests are running

Authorization passed for this commit. See the E2E Tests workflow for results.

@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed by 3-agent squad (Claude, Grok). No blocking findings after verification — all findings downgraded to LOW after confirming design intent with author:

  • Wording: "removing all *:write permissions" reads as stripping scopes entirely in GitHub API terms; implementation will downgrade to read — consider clarifying in the ADR text
  • Migration: mint API default-to-read affects non-harness callers (reconcileMintToken, mint-token CLI, e2e); coordinate in #2823/#2826
  • Phase coverage: validation_loop inherits runtime level — worth a sentence clarifying this is intentional
  • Format detection: levels key as implicit discriminator is fine for now; note as reserved if future forges collide

All cross-references verified (ADRs 0007/0059/0060/0068/0070, issues #2821/#2823/#2826/#5311/#5312, problem docs). Architecture.md updates accurately reflect the ADR.

@ifireball

Copy link
Copy Markdown
Member

/fs-fix deal with the following comments by @waynesun09

Reviewed by 3-agent squad (Claude, Grok). No blocking findings after verification — all findings downgraded to LOW after confirming design intent with author:

* **Wording:** "removing all `*:write` permissions" reads as stripping scopes entirely in GitHub API terms; implementation will downgrade to `read` — consider clarifying in the ADR text

* **Migration:** mint API default-to-`read` affects non-harness callers (`reconcileMintToken`, `mint-token` CLI, e2e); coordinate in #2823/#2826

* **Phase coverage:** `validation_loop` inherits `runtime` level — worth a sentence clarifying this is intentional

* **Format detection:** `levels` key as implicit discriminator is fine for now; note as reserved if future forges collide

All cross-references verified (ADRs 0007/0059/0060/0068/0070, issues #2821/#2823/#2826/#5311/#5312, problem docs). Architecture.md updates accurately reflect the ADR.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 7:14 PM UTC · Completed 7:20 PM UTC
Commit: 02a0757 · View workflow run →

- Clarify "removing *:write permissions" means downgrading to read
  counterparts, not stripping scopes entirely (wording finding)
- Note `levels` key is reserved for format discrimination in
  CUSTOM_ROLE_PERMISSIONS (format detection finding)
- Add sentence clarifying validation_loop inherits runtime level
  intentionally (phase coverage finding)
- Name specific non-harness callers (reconcileMintToken, mint-token
  CLI, e2e) affected by default-to-read, with #2823/#2826 refs
  (migration finding)
- Update docs/architecture.md summary to match wording change

Addresses review feedback on #5328
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 2 (human-triggered)

Addressed all 4 review findings from @waynesun09. Clarified wording on permission downgrading, named specific non-harness callers affected by default-to-read, documented validation_loop phase inheritance, and noted levels key is reserved for format discrimination. Updated both ADR 0073 and docs/architecture.md.

Fixed (4):

  1. *Wording: 'removing all :write permissions' ambiguous in GitHub API terms (docs/ADRs/0073-named-mint-privilege-levels.md): Changed 'removing all *:write permissions' to 'downgrading all *:write permissions to their read counterparts' with an example (contents: write becomes contents: read). Also updated the matching summary in docs/architecture.md.
  2. Migration: mint API default-to-read affects non-harness callers (docs/ADRs/0073-named-mint-privilege-levels.md): Replaced generic 'Direct API callers and mint clients' with explicit mention of reconcileMintToken, mint-token CLI, and e2e test helpers as affected callers. Added coordination reference to mint: implement named privilege levels under agent roles #2823/harness: separate permission profiles per run phase #2826.
  3. Phase coverage: validation_loop inherits runtime level (docs/ADRs/0073-named-mint-privilege-levels.md): Added a sentence in the privilege_levels section clarifying that phases not explicitly listed (such as validation_loop) inherit the runtime level, since they execute within the runtime phase and share its security context.
  4. Format detection: levels key as implicit discriminator could collide (docs/ADRs/0073-named-mint-privilege-levels.md): Added parenthetical note that 'levels' is reserved for format discrimination and role definitions must not use it as a permission name.

Tests: passed

Updated by fullsend fix agent

@maruiz93 maruiz93 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two observations — neither blocking, both worth being mindful of during implementation.

Comment thread docs/ADRs/0073-named-mint-privilege-levels.md Outdated
Comment thread docs/ADRs/0073-named-mint-privilege-levels.md
@ifireball

Copy link
Copy Markdown
Member

/fs-fix adjust ADR 0073 according to the open review threads by @maruiz93:

  1. "Phase" naming collision (docs/ADRs/0073-named-mint-privilege-levels.md ~L79) — Replace harness "phase" terminology with "run-stage" (for pre_script / runtime / post_script) to avoid colliding with other meanings of "phase" in the codebase (security scanning, install progress, ADR rollout milestones, multi-agent dispatch). Update the ADR (and any related wording in docs/architecture.md if present) consistently.

  2. Credential delivery timing (docs/ADRs/0073-named-mint-privilege-levels.md ~L99) — Add a Consequences note that privilege_levels must coordinate token acquisition with credential expansion for both sandbox delivery paths: provider credentials (${GH_TOKEN} expanded at provider resolution) and host files with expand: true. The correct level token must already be in the process env before expansion for the runtime run-stage; pre/post scripts get the write-level token via the host environment.

Resolve the related review conversations once the ADR is updated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 2:38 PM UTC · Completed 2:45 PM UTC
Commit: 3eb6a6e · View workflow run →

…uence

- Replace "phase" terminology with "run-stage" in ADR 0073 and
  architecture.md to avoid naming collision with security scanning,
  install progress, ADR rollout milestones, and multi-agent dispatch
- Add Consequences note on credential delivery timing: privilege_levels
  must coordinate token acquisition with credential expansion for
  provider credentials and host files with expand: true

Addresses review feedback from @maruiz93 on #5328
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 3 (human-triggered)

Addressed both review findings from @maruiz93: (1) replaced all 'phase' terminology with 'run-stage' in ADR 0073 and architecture.md to avoid naming collision, (2) added credential delivery timing Consequences note. Could not resolve PR conversations directly (post-script responsibility).

Fixed (3):

  1. Phase naming collision — replace harness phase terminology with run-stage (docs/ADRs/0073-named-mint-privilege-levels.md): Replaced all instances of 'phase' (referring to pre_script/runtime/post_script) with 'run-stage' throughout the ADR — Context, Clients, Acquisition semantics, Harness privilege_levels flag, and Consequences sections. This avoids collision with other meanings of 'phase' in the codebase (security scanning, install progress, ADR rollout milestones, multi-agent dispatch).
  2. Phase naming collision — update architecture.md consistently (docs/architecture.md): Updated both architecture.md references (Agent Harness section and Agent Identity Provider section) from 'phase'/'run phases' to 'run-stage'/'run-stages', and renamed 'Phase-scoped' to 'Run-stage-scoped'.
  3. Credential delivery timing — add Consequences note (docs/ADRs/0073-named-mint-privilege-levels.md): Added a Consequences bullet noting that privilege_levels must coordinate token acquisition with credential expansion for both sandbox delivery paths: provider credentials (${GH_TOKEN} expanded at provider resolution time) and host files with expand: true. The read-level token must be in the process env before expansion for the runtime run-stage; pre/post scripts get the write-level token via the host environment.

Disagreed (1):

  1. Resolve related review conversations on GitHub: The fix agent cannot post comments or resolve PR review conversations — these are post-script responsibilities. The underlying code changes have been made; conversation resolution requires a human or the post-script to mark the threads as resolved.

Tests: passed

Updated by fullsend fix agent

@ifireball
ifireball added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit beebee5 Jul 23, 2026
14 checks passed
@ifireball
ifireball deleted the agent/5311-named-mint-privilege-levels branch July 23, 2026 14:57
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 23, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 3:00 PM UTC · Completed 3:27 PM UTC
Commit: 9141be0 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5328 — ADR 0073 for named mint privilege levels

Timeline

  1. 2026-07-19 11:12 — ifireball filed #5311, iteratively refining 7 required areas across follow-up comments and 3 triage re-runs
  2. 2026-07-19 17:34 — Code agent (run) created PR docs(#5311): add ADR 0073 for named mint privilege levels #5328 in ~10 min, covering all 7 areas
  3. 2026-07-19 17:45–17:59 — Review agent (run) dispatched 4 sub-agents + challenger, approved with 2 low findings
  4. 2026-07-20 04:03/fs-fix docs: Add agent-compatible code problem document #1 (ifireball) addressing review bot findings + corrections → fix agent (run) ~8 min
  5. 2026-07-20 06:10–13:53 — ifireball and waynesun09 approved; waynesun09's 3-agent squad surfaced 4 additional observations
  6. 2026-07-20 19:13/fs-fix Add problem areas: Tekton pipeline review, migration path, multi-tenancy #2 relaying waynesun09's findings → fix agent (run) ~7 min
  7. 2026-07-22 10:25 — maruiz93 commented with 2 observations (naming collision, credential timing); approved
  8. 2026-07-23 14:37/fs-fix docs: Add codebase context problem document and trim CLAUDE.md #3 relaying maruiz93's findings → fix agent (run) ~8 min
  9. 2026-07-23 14:57 — Merged

What went well

  • Triage quality: Captured all 7 required areas after iterative refinement with follow-up comments
  • Code agent: Produced a solid initial ADR covering all requirements in ~10 min
  • Fix agent efficiency: All 3 fix iterations completed in 7–8 min each, correctly addressing all requested changes
  • Review approval decision: Correct — all human findings were non-blocking observations
  • Human review depth: Three reviewers provided substantive, complementary feedback

Review gap

The review agent found 2 issues (both downgraded to low by the challenger). Human reviewers found 6 additional issues:

  • waynesun09: Wording ambiguity on *:write removal, migration impact on non-harness callers, validation_loop coverage gap, levels key reservation
  • maruiz93: "Phase" naming collision with existing codebase terms, credential delivery timing coordination

The most actionable review-agent finding (read-level derivation rule unspecified, originally medium) was downgraded to low by the challenger using the reasoning "ADR is a decision record, not a specification." ifireball's /fs-fix #1 then explicitly addressed this finding, suggesting medium severity was warranted.

Evidence for existing issues

  • agents#373 (skip correctness/security dimensions for docs-only PRs): This PR provides counter-evidence. The correctness sub-agent was the only dimension that produced actionable findings on this docs-only ADR. The correctness sub-agent's own instructions acknowledge that technical documentation with "algorithm descriptions, API behavior claims" has correctness surface area. If ci: skip site build when no site files changed #373 is implemented, ADR PRs with technical decision content should be excluded from the docs-only dimension skip.

Agents repo

Discovered from review run logs: fullsend-ai/agents at commit 6565264.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: ADR for named mint privilege levels under agent roles

3 participants