Repository navigation
feat(unified-identity): carry the numeric GitHub id in local OAuth JWTs (#374 slice B) #422
Closed as not planned
Description
Parent
Refs #374 — Slice B of the approved design (platform-gitops/agents-state/mctl-api/proposals/issue-374-feat-unified-identity-introduce-a-swappa/tasks.md, section "Slice B"). Slice A shipped in #410 (Refs #374, released in 4.55.0, running in 4.56.0). This issue is the separate follow-up that design calls for; it must not close #374.
Goal
Carry the numeric GitHub id into locally issued OAuth JWTs, so a token resolves to the canonical principal via (github, <numeric id>) without a GitHubIDLookup round-trip, while tokens minted before the change keep working.
Scope (B1 from the design record)
internal/api/oauth_handlers.go(~251): switch fromGitHubValidator.ValidatetoValidateIdentity(internal/auth/github.go).- Thread the id through
IssueCodeandIssueJWT(internal/auth/oauth_server.go) as an optionalghidclaim onjwtPayload. - Teach the Slice A local-OAuth federation provider to prefer
ghidwhen present. - Metric
oauth_jwt_without_github_id_totalfor tokens resolved through the login-only path. - Update
docs/federation.md.
Line numbers are from the design record (2026-09-24) and must be re-read against main.
Acceptance (from the design, test T7)
- A token minted after the change resolves via
(github, <numeric id>)with noGitHubIDLookupcall (internal/principals/resolver.go). - A token minted before the change still resolves through the login-only path, and increments the new metric.
subis still the login; no other claim changes meaning.- Regression test fails if
ghidis dropped fromIssueJWT(mutation-verified).
Out of scope
- Slice C (re-resolve groups on refresh) and Slice D (audience flag day) — separate issues.
- feat(unified-identity): finish canonical agent actor / on-behalf-of identity #376 (agent actor / on-behalf-of) — do not touch agent run tokens.
Activity
mctl-agents commentedon Sep 29, 2026
mctl-agents issue-investigator has analyzed this issue and created a proposal:
https://github.com/mctlhq/mctl-gitops/tree/main/platform-gitops/agents-state/mctl-api/proposals/issue-422-feat-unified-identity-carry-the-numeric/
Status:
proposed— pending human approval. Reviewrequirements.md,design.mdandtasks.md, then approve: signal this issue's DevLoopWorkflow (mctl-apiPOST /api/v1/agents/dev-loop/dev-loop-mctlhq-mctl-api-422/approve, orpython -m orchestrator.temporal.cli approve dev-loop-mctlhq-mctl-api-422 --approver <your-identity>) — the workflow flips.status.yamltoacceptedvia themctl-agents-approveoperation and runs the Tier 2 implementer. If no DevLoopWorkflow is running for this issue (pre-Temporal proposal), run themctl-agents-approveoperation directly withservice=mctl-api slug=issue-422-feat-unified-identity-carry-the-numeric.The approver is recorded in
.status.yamland in the gitops commit, and the implementer refuses a proposal whosecontrol.requires_human_approvalcarries no named approver — so prefer the mctl-api endpoint, which takes the approver from the authenticated caller rather than from what the caller types.chore(agents): investigate mctlhq/mctl-api#422 2026-09-29
mashkovd commentedon Sep 29, 2026
Proposal review (not approved — waits for the owner)
Checked
issue-422-feat-unified-identity-carry-the-numericagainst mctl-apimain.What holds up. Scope is slice B only (no slice C/D, no #376 run tokens).
ghidisomitemptyandsubstays the login, so old verifiers and old tokens are unaffected. Explicit new parameters onIssueCode/IssueJWT/IssueRefreshTokenmake every call site a compile error until considered. T1 is a real mutation pin; T4 reuses the existing characterization harness for both middleware chains.P2 — in production the benefit lasts one hour per login. The proposal mints refreshed access tokens without
ghidon therefreshstore(Postgres) path and records the column as a follow-up. But production uses that path: mctl-api 4.56.0 logsoauth refresh token store initializedat boot,AccessTokenTTLis 1h andRefreshTokenTTL30d (internal/auth/oauth_server.go:767-768). So a Claude/ChatGPT MCP client carriesghidonly on its first access token; every token after the first refresh — i.e. almost all traffic — falls back to the login-only path and theGitHubIDLookupthe issue is meant to remove. The acceptance line "a token minted after the change resolves via(github, <numeric id>)" is then false for refreshed tokens.Suggested resolution (owner's choice):
github_id BIGINTcolumn on the refresh-token table in this slice, written atIssueRefreshTokenand read inRefreshAccessToken; existing rows stay NULL and keep the login-only fallback, so the migration is additive and needs no backfill; ordocs/federation.mdthat production coverage is first-token-only until it lands.P3.
oauth_jwt_without_github_id_totalcounts per validation, so after rollout it will be dominated by refreshed tokens (see P2) rather than by genuinely pre-change tokens; with (a) it becomes a clean "old tokens still in circulation" signal that should decay to zero within 30 days.Status:
proposed, not approved.mashkovd commentedon Oct 17, 2026
Superseded by #377 PR-B 2a (
36f1a21). Local OAuth sessions now carry the canonical principal directly:pidin access tokens andprincipal_idin the refresh row, preserved across refresh rotation. In principal mode requests authorize directly by that principal, so carrying a numeric GitHub ID in the JWT no longer removes an additional lookup or improves the authorization path.ValidateIdentityis already used at GitHub sign-in. Closing as not planned.