Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical flaws could expose learning-store files without authentication, permit stale or unauthorized connection changes, and exhaust resources through unbounded concurrent CLI checks.
15 open issues
Recheck administrator authority after acquiring the state lock · New Exclude the configured client directory from learning roots · New Limit concurrent connection checks before spawning processes · New Advance connection generation when recording a refusal · New Limit form interception to connection forms · New Add direct tests for disconnect authorization and audit behavior · New Preserve a specific learning configuration startup error · New Stop surviving child processes after the parent exits · New Add PostgreSQL connection persistence tests and reseed state · New Map transaction conflicts to connection conflict responses · New Test all connection audit phases through the presentation pipeline · New Parse connection request bodies after origin checks · New Wrap each documentation paragraph and list item on one line · New Use Prisma relation filters for administrator existence checks · New Move refusal state and transaction decisions into the domain · New
What changed in this PR
Adds the foundation for Hub to use the existing learning package, without introducing repository learning runs or product records that are not yet available.
Changes:
- Connects the administrator Copilot settings page to real APIs, durable connection state, and typed security audits.
- Adds bounded CLI execution, private-store preparation, and a PostgreSQL execution-lock primitive.
- Includes the learning package in the Hub image and updates configuration, documentation, and tests.
| File | Description |
|---|---|
hub/test/fixtures/learning/report-runtime.mjs |
Reports subprocess environment and temporary-directory permissions. |
hub/test/fixtures/learning/hang-with-child.mjs |
Provides a process-tree timeout fixture. |
hub/server/utils/validate-private-storage-root/index.ts |
Validates private storage paths. |
hub/server/start-server-processes/index.ts |
Accepts Copilot route dependencies. |
hub/server/run-api/index.ts |
Passes Copilot dependencies into the app. |
hub/server/routes/types.ts |
Extends shared route dependencies. |
hub/server/routes/index.ts |
Registers connection status and mutation endpoints. |
hub/server/routes/api/security-audits/present-security-audit/index.ts |
Presents Copilot audit actors and context. |
hub/server/routes/api/copilot-connection/types.ts |
Defines connection route dependencies. |
hub/server/routes/api/copilot-connection/read-session-digest/index.ts |
Reads and hashes the session cookie. |
hub/server/routes/api/copilot-connection/index.test.ts |
Covers mutation authorization and confirmation. |
hub/server/routes/api/copilot-connection/get/index.ts |
Returns administrator-only connection status. |
hub/server/routes/api/copilot-connection/disconnect/delete/index.ts |
Handles confirmed disconnections. |
hub/server/routes/api/copilot-connection/create-copilot-connection-route-dependencies/index.ts |
Composes verification, persistence, and auditing. |
hub/server/routes/api/copilot-connection/contract.ts |
Defines the status response schema. |
hub/server/routes/api/copilot-connection/connect/post/index.ts |
Handles confirmed connection checks. |
hub/server/repositories/security-audits/parse-security-audit-event/types.ts |
Adds the Copilot audit event type. |
hub/server/repositories/security-audits/parse-security-audit-event/index.ts |
Validates Copilot audit events. |
hub/server/repositories/security-audits/get-security-audit-data/index.ts |
Maps connection phases to stored audit records. |
hub/server/repositories/security-audits/create-security-audit/types.ts |
Defines typed connection audit inputs. |
hub/server/repositories/memberships/is-effective-administrator/index.ts |
Checks current administrator authority. |
hub/server/repositories/copilot-connections/replace-copilot-connection/index.ts |
Replaces the deployment-wide connection. |
hub/server/repositories/copilot-connections/refuse-copilot-connection/index.ts |
Records token refusal state. |
hub/server/repositories/copilot-connections/get-copilot-connection/index.ts |
Loads connection and administrator details. |
hub/server/repositories/copilot-connections/delete-copilot-connection/index.ts |
Deletes the saved connection. |
hub/server/repositories/copilot-connection-states/lock-copilot-connection-state/index.ts |
Locks the persistent generation row. |
hub/server/repositories/copilot-connection-states/advance-copilot-connection-state/index.ts |
Advances the connection generation. |
hub/server/integrations/postgres/with-learning-execution-lock/index.ts |
Adds deployment-wide execution locking. |
hub/server/integrations/postgres/with-learning-execution-lock/index.db.test.ts |
Covers lock contention, release, and connection loss. |
hub/server/integrations/learning/run-learning-command/index.ts |
Runs the CLI with execution and output limits. |
hub/server/integrations/learning/run-learning-command/index.test.ts |
Covers environment isolation and timeout cleanup. |
hub/server/integrations/learning/run-learning-command/errors.ts |
Defines typed command errors. |
hub/server/index.ts |
Validates learning configuration and wires routes. |
hub/server/index.test.ts |
Covers Copilot dependency composition at startup. |
hub/server/domains/learning/read-learning-configuration/index.ts |
Resolves learning paths and token configuration. |
hub/server/domains/learning/read-learning-configuration/index.test.ts |
Covers public-root and symlink rejection. |
hub/server/domains/learning/prepare-learning-store/index.ts |
Prepares private repository stores and configuration. |
hub/server/domains/learning/prepare-learning-store/index.test.ts |
Covers store creation and permissions. |
hub/server/domains/copilot-connection/types.ts |
Defines snapshots and transaction repository contracts. |
hub/server/domains/copilot-connection/get-copilot-connection-status/index.ts |
Derives status from token and authority state. |
hub/server/domains/copilot-connection/get-copilot-connection-status/index.test.ts |
Covers token replacement, authority loss, and refusal. |
hub/server/domains/copilot-connection/errors.ts |
Defines connection operation errors. |
hub/server/domains/copilot-connection/disconnect-copilot/index.ts |
Deletes connections with generation and audit updates. |
hub/server/domains/copilot-connection/connect-copilot/index.ts |
Verifies tokens and guards connection changes. |
hub/server/domains/copilot-connection/connect-copilot/index.test.ts |
Covers authority, stale snapshots, and persistence inputs. |
hub/server/create-app/index.ts |
Wires Copilot routes and security headers. |
hub/script/check-image/index.ts |
Uses the root build context and checks SDK inclusion. |
hub/schema/migrations/20261110220000_copilot_connection_foundation/migration.sql |
Creates connection tables, constraints, and generation seed. |
hub/schema/membership.prisma |
Adds the connection membership relation. |
hub/schema/copilot-connection.prisma |
Models the saved deployment-wide connection. |
hub/schema/copilot-connection-state.prisma |
Models persistent connection generation. |
hub/docs/README.md |
Links the learning foundation guide. |
hub/docs/learning-foundation.md |
Documents boundaries, configuration, and deferred integration. |
hub/docs/frontend.md |
Documents the real Copilot settings route. |
hub/docs/development.md |
Explains local configuration and startup validation. |
hub/docs/deployment.md |
Updates image build and publishing commands. |
hub/docs/architecture.md |
Describes the connection and execution foundation. |
hub/Dockerfile.dockerignore |
Defines exclusions for the root build context. |
hub/Dockerfile |
Installs and includes the learning CLI and SDK. |
hub/client/pages/copilot-connection-page/stories.tsx |
Updates connection states and confirmation copy. |
hub/client/pages/copilot-connection-page/index.test.tsx |
Updates presentation and form expectations. |
hub/client/application/hub/view-models/copilot-connection-page-data/index.ts |
Maps live status to page controls and feedback. |
hub/client/application/hub/view-models/copilot-connection-page-data/index.test.ts |
Covers configured and missing-token controls. |
hub/client/application/hub/view-models/administrator-workspace/index.ts |
Adds Copilot settings navigation. |
hub/client/application/hub/routes/settings-copilot/index.tsx |
Connects loaders, actions, and mutation feedback. |
hub/client/application/hub/routes/administrator-flow.test.tsx |
Exercises real-mode Copilot API integration. |
hub/client/application/hub/requests/get-copilot-connection/index.ts |
Requests validated connection status. |
hub/client/application/hub/requests/change-copilot-connection/index.ts |
Sends confirmed connection mutations. |
hub/client/application/hub/is-administrator-route/index.ts |
Classifies Copilot settings as an administrator route. |
hub/client/application/hub/is-administrator-route/index.test.ts |
Covers Copilot route variants. |
hub/client/application/hub/copilot-connection-contracts/types.ts |
Exports the client status type. |
hub/client/application/hub/copilot-connection-contracts/index.ts |
Validates client status responses. |
hub/.env.example |
Lists optional learning configuration. |
hub/.dockerignore |
Removes exclusions superseded by the Dockerfile-specific file. |
Review effort: Max (auto)
|
What account would the Copilot run under? |
2af837d to
0106fac
Compare
Good question. It currently uses the admin account which is parity with what's in main and is still useful for running locally. In a follow up PR I will add the ability to charge the org with the new copilot funtionality. |
There was a problem hiding this comment.
🟡 Changes recommended
The Docker context includes .test-work, potentially exposing local learning stores or exports containing review evidence in the published runtime image.
6 open issues
Exclude .test-work from the Docker context · New Keep credential checks available while connected · New Show Disconnect for inactive saved connections · New Ignore storage and exclude it from the Docker context · New Abort and clean up Copilot checks before Hub shutdown · New Rename credentialProvider in audit responses · New
15 resolved since last review
Advance connection generation when recording a refusal Limit concurrent connection checks before spawning processes Exclude the configured client directory from learning roots Recheck administrator authority after acquiring the state lock Parse connection request bodies after origin checks Test all connection audit phases through the presentation pipeline Map transaction conflicts to connection conflict responses Add PostgreSQL connection persistence tests and reseed state Stop surviving child processes after the parent exits Preserve a specific learning configuration startup error Add direct tests for disconnect authorization and audit behavior Limit form interception to connection forms Move refusal state and transaction decisions into the domain Use Prisma relation filters for administrator existence checks Wrap each documentation paragraph and list item on one line
Review effort: Max (auto)
678c6f5 to
910a55f
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Critical issues can expose private learning data in runtime images and allow connection changes using concurrently revoked administrator sessions.
10 open issues
Allowlist learning runtime inputs in Docker context · New Revalidate session authority after lock waits · New Make alternate registry retries opt-in · New Revalidate loaders after refused rechecks · New Log allowlisted codes for Copilot failures · New Follow symlinks when canonicalizing static exclusions · New Align the Refused story with reachable UI actions · New Install learning dependencies for local Copilot use · New Assert the credential provider in disconnect audits · New Cover subprocess output limits with regression tests · New
6 resolved since last review
Review effort: Max (auto)
ef7d775 to
c779b0f
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
c779b0f to
389e5ed
Compare



Why
Hub cannot run the existing explicit learning loop until it has a secure deployment-wide Copilot connection, durable connection state, and a safe boundary for invoking the host-neutral learning package. This adds that foundation without inventing product records that the rebuilt Hub does not have yet.
What changes
Hub administrators can view, connect, recheck, and disconnect an operator-configured Copilot credential through the retained Copilot settings page. The connection stores a provider key and stable configuration digest rather than a credential or short-lived access-token identity. The included
ambient-cliandenvironment-tokenproviders are explicit local/backward-compatible modes; no personal credential is the production default.The provider supplies an explicit credential for each learning command, which supports future short-lived credential rotation without changing the connection lifecycle or CLI boundary. The command runs as a separate process with an allowlisted environment, private temporary state, bounded output, a deadline, and process-group cleanup. The image installs the separate learning package and Copilot SDK. Private learning-root validation, repository-store preparation, and a PostgreSQL execution lock provide the storage and serialization seams for later repository runs.
Repository learning runs, review import, decisions, redaction, and recall remain separate work until real repository, Build, prototype, and review-comment APIs exist.
packages/learningcontinues to own consent and learning lifecycle rules.Organization-attributed GitHub App authentication remains a separate provider implementation: it needs the eligible Copilot SDK upgrade, App and installation configuration, short-lived installation-token minting and retry, organization billing and revocation behavior, and corresponding audit and UI attribution.
Storybook screenshots
Hub/Pages/Security/Copilot — Not Connected · dark · 1280×720
Hub/Pages/Security/Copilot — No Credential · dark · 1280×720
Hub/Pages/Security/Copilot — Connected · dark · 1280×720
Hub/Pages/Security/Copilot — Refused · dark · 1280×720
Hub/Pages/Security/Copilot — Access Removed · dark · 1280×720