diff --git a/docs/adr/0001-learning-extension-architecture.md b/docs/adr/0001-learning-extension-architecture.md index 51c3781b..dd4bb6ff 100644 --- a/docs/adr/0001-learning-extension-architecture.md +++ b/docs/adr/0001-learning-extension-architecture.md @@ -96,7 +96,7 @@ Each import records whether every list page and reply page was present. The impo Hub hosts the loop and runs the `replica-learning` command for every step, so the store's rules stay the only rules. Hub keeps one store per repository in its own storage, outside any checkout. A background job imports the repository's review comments from Hub's database as one complete collection, removes the text of comments deleted on Hub, asks Copilot to label each new or edited comment, and consolidates the labels into proposals. When someone deletes a comment, another job removes its text by its Hub comment ID, which imports nothing, so it works without an active grant and adds no other comments. Repository administrators accept, edit, or reject proposals on a Learnings page. The Replica prototype plugin adds accepted learnings to the prompts a person submits in that repository. [Learnings](../../hub/docs/learnings.md) describes the product. -Hub asks Copilot through the [Copilot SDK](https://github.com/github/copilot-sdk), with a token that the person who runs Hub provides and whose only permission is Copilot requests, as [Copilot](../../hub/docs/copilot.md) describes. The session has no tools, refuses every permission request, and never sees who wrote a comment. Copilot chooses the model unless the store's configuration names one, and each run records the usage Copilot reports. +Hub asks Copilot through the [Copilot SDK](https://github.com/github/copilot-sdk), with a token whose only permission is Copilot requests: one that the person who runs Hub provides, or, with organization billing, a one-hour token that Hub's GitHub App creates for each request, as [Copilot](../../hub/docs/copilot.md) describes. The session has no tools, refuses every permission request, and never sees who wrote a comment. Copilot chooses the model unless the store's configuration names one, and each run records the usage Copilot reports. The command still runs in a checkout, with data under `.replica/learning/`, for contributors and for other hosts. This complements #230: its discovery output can become labels, and its judge can become a consolidator. @@ -114,7 +114,7 @@ Learning happens in explicit, inspectable memory. A training-purpose export is t - The generated validator file is large, as in `packages/controls`. - Hub keeps one SQLite file per repository. A team that needs a shared database server brings its own provider and runs the conformance suite against it. - Hub's production image includes Node.js and the command, which adds about 260 MB. -- Copilot usage counts toward the token owner's plan, not necessarily the administrator who enabled the connection. The operator supplies a token intended to have only the Copilot requests permission; Hub keeps it in process configuration, not its database. The operator must verify its actual scope and manage expiry, rotation and revocation. Token replacement requires a full stop/start with no old/new process overlap, including learning and SDK children; rolling replacement is unsupported. Credential custody, subprocess isolation and deployed authentication still need security review. +- By default, Copilot usage counts toward the token owner's plan, not necessarily the administrator who enabled the connection. With organization billing, the organization that installed Hub's GitHub App pays, and anyone who controls the App's private key, which Hub already stores, can spend that organization's Copilot budget. The operator supplies a token intended to have only the Copilot requests permission; Hub keeps it in process configuration, not its database. The operator must verify its actual scope and manage expiry, rotation and revocation. Token replacement requires a full stop/start with no old/new process overlap, including learning and SDK children; rolling replacement is unsupported. Credential custody, subprocess isolation and deployed authentication still need security review. ## Alternatives considered @@ -124,7 +124,7 @@ Learning happens in explicit, inspectable memory. A training-purpose export is t - Dropping records that fail their checks and continuing. Rejected because dropping a damaged withdrawal or retraction silently restores a permission. - A knowledge graph or vector store as the default. Rejected; the default should be simple and local, and graph or semantic search can arrive as provider capabilities. - An agent collects comments through the canvas, and a starter skill runs each step. Replaced: a person had to start every step, and every comment passed through an agent session, which cost tokens and could introduce copying errors. -- Billing Copilot to the organization through the App's installation token. Deferred: it needs the App's Copilot requests permission and an installation on every repository in the organization. +- Organization billing through the App's installation tokens as the only option. Rejected: it needs Copilot Business or Copilot Enterprise and an App installed on an organization, and GitHub offers it as a preview. Hub offers it alongside the operator's token. - PostgreSQL or Neo4j providers in this version. Deferred: SQLite meets one Hub's needs, and the provider interface and conformance suite let a team add one later. - Model-only consolidation. Kept as a plug-in option; the reference stays deterministic so tests and reviews are reproducible. diff --git a/hub/.env.example b/hub/.env.example index 80e5ecb6..52e27a01 100644 --- a/hub/.env.example +++ b/hub/.env.example @@ -14,6 +14,9 @@ REPLICA_ARTIFACT_ROOT=storage/artifacts # Learnings asks Copilot with this token: a fine-grained personal access token whose only # permission is Copilot requests. Hub never stores it. See docs/copilot.md. # REPLICA_COPILOT_TOKEN= +# To bill the organization that installed Hub's GitHub App instead of the token's owner, give the +# App the Copilot requests organization permission and set this. Hub then ignores the token. +# REPLICA_COPILOT_BILLING=organization # Use a non-empty secret. A long random value is recommended for public deployments. REPLICA_BOOTSTRAP_SECRET= # Production deployments should set independent random values for these keys. diff --git a/hub/app/controllers/copilot_connections_controller.rb b/hub/app/controllers/copilot_connections_controller.rb index dfbbb1c0..a70483fc 100644 --- a/hub/app/controllers/copilot_connections_controller.rb +++ b/hub/app/controllers/copilot_connections_controller.rb @@ -1,12 +1,13 @@ -# Lets Hub administrators connect Copilot with the operator's token, see who connected it, and -# disconnect it. Both changes need fresh GitHub proof, like Hub's other administrator settings. -# Hub keeps no Copilot credential; see CopilotConnection. +# Lets Hub administrators connect Copilot with the credential the operator configured, see who +# connected it, and disconnect it. Both changes need fresh GitHub proof, like Hub's other +# administrator settings. Hub keeps no Copilot credential; see CopilotConnection. class CopilotConnectionsController < SuperglueController include FreshAuthorizationRequired before_action :require_github_hub before_action :authorize_admin - before_action :confirm_change, only: %i[create destroy] + before_action :confirm_connect, only: :create + before_action :confirm_disconnect, only: :destroy before_action :authorize_connect, only: :create before_action :authorize_disconnect, only: :destroy @@ -21,16 +22,20 @@ def show # @route POST /copilot (copilot_connection) def create - CopilotConnection.connect!(actor: Current.user, browser_session: Current.session) + CopilotConnection.connect! \ + actor: Current.user, + browser_session: Current.session, + credential: @confirmed_credential @security_audit_recorded = true redirect_to copilot_connection_path, notice: t("copilot_connection.connected"), status: :see_other rescue CopilotConnection::Refused, Learning::Error => error reason, outcome = case error + when CopilotConnection::PermissionMissing then [ :permission_missing, "denied" ] when CopilotConnection::Refused then [ :refused, "denied" ] else [ error.code == "not_installed" ? :not_installed : :unavailable, "failed" ] end record_security_audit(outcome:) - redirect_to copilot_connection_path, alert: t("copilot_connection.errors.#{reason}"), status: :see_other + redirect_to copilot_connection_path, alert: helpers.copilot_billing_t("errors.#{reason}"), status: :see_other end # @route DELETE /copilot (copilot_connection) @@ -42,7 +47,7 @@ def destroy private - # Local development has no Copilot token, so it uses the developer's own Copilot sign-in instead. + # Local development has no Copilot credential, so it uses the developer's own Copilot sign-in instead. def require_github_hub head :not_found if LocalDevelopment.enabled? end @@ -57,7 +62,13 @@ def authorize_admin render :show, status: :forbidden end - def confirm_change + def confirm_connect + return change_denied(CopilotConnection::ChangeDenied.new(:confirmation_required)) if params[:confirmed] != "1" + + @confirmed_credential = CopilotConnection.confirmed_credential!(params[:billing_confirmation]) + end + + def confirm_disconnect return if params[:confirmed] == "1" change_denied(CopilotConnection::ChangeDenied.new(:confirmation_required)) @@ -85,7 +96,7 @@ def change_denied(error) else redirect_to \ copilot_connection_path, - alert: t("copilot_connection.errors.#{error.reason}"), + alert: helpers.copilot_billing_t("errors.#{error.reason}"), status: :see_other end end diff --git a/hub/app/helpers/copilot_connections_helper.rb b/hub/app/helpers/copilot_connections_helper.rb new file mode 100644 index 00000000..8f3347a5 --- /dev/null +++ b/hub/app/helpers/copilot_connections_helper.rb @@ -0,0 +1,10 @@ +# Describes the Copilot credential the operator configured, in the words that fit who Copilot bills. +module CopilotConnectionsHelper + # Translates key under scope. When Copilot bills the organization that installed Hub's GitHub App, + # the organization's wording under scope.organization replaces the token owner's where it differs. + def copilot_billing_t(key, scope: "copilot_connection", **options) + return t("#{scope}.#{key}", **options) if !CopilotConnection.organization_billing? + + t("#{scope}.organization.#{key}", default: :"#{scope}.#{key}", **options) + end +end diff --git a/hub/app/javascript/pages/copilot-connection-page/index.test.tsx b/hub/app/javascript/pages/copilot-connection-page/index.test.tsx index 2b49eada..3b294f50 100644 --- a/hub/app/javascript/pages/copilot-connection-page/index.test.tsx +++ b/hub/app/javascript/pages/copilot-connection-page/index.test.tsx @@ -18,6 +18,11 @@ describe("CopilotConnectionPage()", () => { it("asks for confirmation before connecting and says who pays for the usage", () => { renderPage(story.args.data) expect(screen.getByRole("heading", { name: "Copilot is not connected" })).toBeInTheDocument() + expect( + screen.getByText( + "Copilot counts the usage toward the Copilot plan of the person who owns the token in REPLICA_COPILOT_TOKEN.", + ), + ).toBeInTheDocument() expect( screen.getByText(/The usage counts toward the Copilot plan of the token's owner/), ).toBeInTheDocument() @@ -31,6 +36,9 @@ describe("CopilotConnectionPage()", () => { expect(form).toHaveAttribute("method", "post") expect(form?.querySelector('[name="_method"]')).toBeNull() expect(form?.querySelector('[name="authenticity_token"]')).toHaveValue("storybook-token") + expect(form?.querySelector('[name="billing_confirmation"]')).toHaveValue( + "signed-billing-confirmation", + ) expect(screen.queryByRole("button", { name: "Disconnect Copilot" })).not.toBeInTheDocument() }) diff --git a/hub/app/javascript/pages/copilot-connection-page/index.tsx b/hub/app/javascript/pages/copilot-connection-page/index.tsx index a2704911..f349e8fe 100644 --- a/hub/app/javascript/pages/copilot-connection-page/index.tsx +++ b/hub/app/javascript/pages/copilot-connection-page/index.tsx @@ -6,7 +6,7 @@ import { HubAdministrationPage } from "@features/hub-administration-page" import type { CopilotConnectionPageData } from "./types" import "./styles.css" -/** Shows who connected Copilot, whether Hub has a token, and each change's consequence beside its confirmation. */ +/** Shows who connected Copilot, who Copilot bills, and each change's consequence beside its confirmation. */ export function CopilotConnectionPage({ data }: { data: CopilotConnectionPageData }) { return ( @@ -27,7 +27,8 @@ export function CopilotConnectionPage({ data }: { data: CopilotConnectionPageDat ) : ( {data.emptyLabel} )} - {data.tokenLabel ? : null} + {data.billingLabel ? {data.billingLabel} : null} + {data.setupLabel ? : null} {data.refusedLabel ? : null} @@ -41,6 +42,11 @@ export function CopilotConnectionPage({ data }: { data: CopilotConnectionPageDat className="copilot-connection-page__form" > +