Key Remote Control credential by GitHub identity (shared per-user across triage + chat)#144
Merged
Merged
Conversation
The `remote-control-url` content chunk only had a trailing blank line, so when an earlier streamed chunk (e.g. agent-text) didn't end in a newline the link ran together with it into one mangled line in the chat client. Add a leading blank line too so the link always stands alone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017LbwJwiShKRbRVQYinnMaJ
A first, marker-less turn (e.g. a chat request to file an issue) has no repo before it runs -- the agent clones during the turn -- so `priorHeadSha` is null. The old `madeNewCommits = headSha !== null && headSha !== priorHeadSha` then read as true purely because a repo now existed with a non-null HEAD, even though the agent only READ the repo (cloned it, opened a GitHub issue via `gh`, committed nothing). With no PR on that branch, this posted a spurious "⚠️ Work is on <repo> branch <branch>, but no open pull request was found." warning on top of a task that actually succeeded. Distinguish the two cases: when a prior HEAD was captured (the repo existed before the turn -- a continuation, including review-only turns), keep diffing against it. When it was not (repo cloned during the turn), a non-null HEAD is just the clone's base commit; instead ask whether HEAD moved past the clone's default-branch tip (`origin/HEAD..HEAD`) via the new `countCommitsAheadOfOriginHead`. A read-only turn leaves HEAD exactly at origin/HEAD (count 0 -> no warning); a turn that genuinely committed and opened a PR still shows the ✅ line, and one that committed without a PR still warns. Adds real-git integration tests for the new helper (clone-and-read -> 0, commits added -> counted, non-repo path -> 0 rather than throwing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017LbwJwiShKRbRVQYinnMaJ
… across flows The `claude-remote` (Remote Control login) credential was keyed by the raw per-caller `identity.subject`. But the same human reaches it through two entry points that resolve DIFFERENT subjects: Open WebUI chat (`openwebui:<id>`, from the per-user JWT) vs the GitHub-triage relay (a shared service-token subject). So a credential linked in one flow was invisible to the other, and the user was re-prompted to `claude login` when switching between triage and chat. Key it instead by the ONE identity both flows can resolve for the same human: their GitHub login (`github:<login>`), read from the caller's already-linked GitHub credential (the RC session already acts as that GitHub user). A new `linkSubjectFor` helper applies this to every `claude-remote` gateway call (getToken/start/poll/waitForCompletion/invalidate, plus the reported pending-link subject) across the delegate gate, the pending-link resume, the turn-failure invalidation, and the agent-backed-tool path. This stays strictly per-user -- distinct GitHub users get distinct keys, never a single shared account (NOT the earlier global-subject approach, which was the wrong reading of "shared"). It only recognises the same user across flows. When no GitHub login can be resolved (no GitHub link yet, or no GitHub gateway), it falls back to the raw subject: still per-user, just not yet cross-flow -- and the RC agent requires GitHub for repo writes anyway, so the link is normally already present. Also re-lands two fixes that were stranded on the PR #141 branch and never reached main (the merge only took the first commit): the chat Remote Control link now renders as its own paragraph, and a turn that made no commits (e.g. a chat request that only filed an issue) no longer gets a false "no open pull request was found" warning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017LbwJwiShKRbRVQYinnMaJ
imaustink
added a commit
that referenced
this pull request
Jul 25, 2026
Principals: Identity gains a `principal` -- a stable id for the HUMAN, independent of entry point -- resolved ONCE in resolveIdentity and read everywhere downstream. Entry-point subjects become aliases; durable per-user state (the Claude credentials) keys on the principal, so one authorization covers GitHub triage and Open WebUI chat. This retires resolveCredentialSubject, whose per-provider re-derivation was the exact shape of the PR #144 bug: many call sites each deciding a key. Deliberately does NOT rewrite identity.subject itself. Doing so would move session keys, RBAC scoping and credential keys simultaneously; feeding the verified login into principal resolution instead lets sessions and RBAC stay on the subject they have always used while only credentials converge. Signed sender assertion: the gateway authenticates to /invoke with its own service token, which says the gateway is calling and nothing about the human. The sender login travelled as an unsigned body field -- yet it selects the caller's principal and therefore which stored credentials a run receives, so anything holding that token could name an arbitrary login and be handed that person's credentials. It is now an HMAC-SHA256 assertion the orchestrator verifies; with the secret configured the body field is ignored outright. Compact HMAC rather than JWT: both ends are in this repo, the only claims are a login and an expiry, and node:crypto avoids adding a JWT library to the gateway to agree with the orchestrator's. Without the secret both processes warn loudly and fall back to the old behaviour, so upgrading does not silently break a deployment nor silently stay weak. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LWBX7XTfAY66wFk73q8kX9
imaustink
added a commit
that referenced
this pull request
Jul 25, 2026
The property §1 asks for already held: authorization ran as graph control flow on the only path to a launch, never as something the planner could select. What it lacked was a boundary -- "where is authorization decided" was answered by reading ~300 lines inside delegateToAgent. graph.ts loses 345 lines; the full pre-existing orchestrator suite passes unchanged. Two things fell out that the "purely cosmetic refactor" framing missed. The verdict is now a total discriminated union -- authorized | link-required | misconfigured -- so a fourth outcome breaks compilation at the branch rather than falling through to "launch anyway". That is the failure direction that matters for a gate. And the agent-backed-tool path had its OWN hand-copied provider loop with the same principal-vs-subject keying rules. Two copies of credential keying is precisely the shape of the #144 bug. It now calls the same owner through a deliberately separate read-only entry point, resolveLinkedCredentials: that path must never START a link, because a paused *tool* call has no resume slot, unlike a paused agent delegation. A boolean flag on authorize() was the alternative and rejected -- it would make "does calling this begin an OAuth flow?" depend on an argument, which is the ambiguity §1 exists to remove. 23 unit tests cover what the extraction made directly reachable, including verdicts previously observable only through a launched AgentRun: the batch prompt for two missing links, a failed start reported ALONGSIDE a successful one, the write-back grant keyed to the same subject the credential was read from, and the degradation to pending when the long-held wait throws. Also drops the three [identity-gate-debug] console.logs, which were marked "remove once root-caused" -- this PR root-causes the 401. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LWBX7XTfAY66wFk73q8kX9
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.
Why
Testing surfaced that after linking Claude in the triage flow, the chat flow re-prompted for auth despite the same user having already linked. Root cause: the
claude-remote(Remote Control login) credential was keyed by the raw per-calleridentity.subject, and the two flows resolve different subjects for the same human:openwebui:<id>— from the per-user signed JWT.So a credential linked in one flow was invisible to the other.
Fix
Key
claude-remoteunder the one identity both flows can resolve for the same human — their GitHub login (github:<login>), read from the caller's already-linked GitHub credential (the RC session already acts as that GitHub user). A newlinkSubjectForhelper applies this to everyclaude-remotegateway call —getToken/start/poll/waitForCompletion/invalidate, plus the reported pending-link subject — across the delegate gate, the pending-link resume, turn-failure invalidation, and the agent-backed-tool path.Strictly per-user — distinct GitHub users → distinct keys, never one shared account. (This is deliberately not the earlier global-subject idea, which was a misread of "shared" and never reached main.) When no GitHub login can be resolved (no GitHub link yet, or no GitHub gateway), it falls back to the raw subject: still per-user, just not yet cross-flow — and the RC agent needs GitHub for repo writes anyway, so the link is normally already present.
Also re-lands two stranded fixes
The PR #141 merge only took its first commit; these two never reached
main:origin/HEADwhen the repo was cloned during the turn.Testing
agent-orchestrator: 340 tests pass, typecheck clean. Added a test assertingclaude-remotekeys undergithub:<login>whileclaude(setup-token) stays per-caller.claude-code-swe-agent: 23 tests pass, typecheck clean (includes the re-landed real-git warning tests).Rollout note
Any credential already linked under an old per-flow subject won't be found under the new
github:<login>key, so the first request after deploy re-links once, then it's shared from both flows permanently.🤖 Generated with Claude Code
https://claude.ai/code/session_017LbwJwiShKRbRVQYinnMaJ