diff --git a/packages/qa/cases/cross-surface/github-settings-connection-panel.md b/packages/qa/cases/cross-surface/github-settings-connection-panel.md index 543183bfd..d2682723f 100644 --- a/packages/qa/cases/cross-surface/github-settings-connection-panel.md +++ b/packages/qa/cases/cross-surface/github-settings-connection-panel.md @@ -101,16 +101,31 @@ only touches the webhook where an `installation.created` delivery records the ro ### Web UI (Settings → GitHub) -- Admin, connected: the connected account + type, "Connection" and "Source repos" section headings, and the - "Manage connection" / "Manage on GitHub" / "Connection details" controls; the connect panel exposes Reinstall (step 1, - when installed) and Disconnect (step 2). +- Admin, connected: the connected account + type, the "Connection" / "Automatic handling" / "Repositories" section + headings, and the "Manage connection" / "Manage on GitHub" / "Connection details" controls; the connect panel exposes + Reinstall (step 1, when installed) and Disconnect (step 2). The Repositories section hands off to Settings → + Repositories rather than editing repository URLs here. - Admin, not connected: a "Connect GitHub" call-to-action. +- Connection details, permissions satisfied: every entry under "Required for automatic replies" reads as ready, and no + GitHub-side call to action appears. The heading stays scoped to that capability — it must not claim the installation + satisfies First Tree generally, since Context Reviewer and repository coverage gate on their own requirements. +- Connection details, a required permission missing (downgrade `issues` or `pull_requests` to `read` on the installation + row): the entry reads as blocked and names what it costs, a "Grant on GitHub" action appears for an admin, and the + shortfall is also visible on the *collapsed* disclosure. This must agree with the server: the same installation makes + `team-agent` assignment fail with `github_app_task_reply_permission_required`, so the readout and the gate never + disagree. +- Connection details, facts: subscribed events First Tree does not consume are listed as unused rather than blended into + the consumed list; `installation` / `installation_repositories` are *not* among them, since the webhook route consumes + both through `handleInstallationLifecycle` — a normally configured installation must never be described as carrying + unused subscriptions. The installation timestamp is labelled "First seen", not "Connected": the row is written unbound + by `installation.created` and survives disconnect/rebind, so it does not record when this team connected. The + installation id copies (a non-secure-context copy failure surfaces "Copy failed" instead of looking inert). - Google/OIDC admin without GitHub identity: after opening Connect, only the inline link-account state appears. After the identity is linked and the same Team is restored, the panel shows the linked login and Install remains a separate click. - Install preflight mismatch: the current First Tree session and selected Team remain unchanged; the recovery message names the expected GitHub login and offers a retry rather than opening the picker. -- Member: the connection state stays readable, but every admin control (Manage / Disconnect / Connect / Reinstall / - Install / Add source repo) is absent. +- Member: the connection state and the permission diagnosis stay readable, but every admin control (Manage / Disconnect / + Connect / Reinstall / Install / Grant on GitHub) is absent. - No browser console errors on any state. ## Expected Result diff --git a/packages/server/src/services/team-agent-settings.ts b/packages/server/src/services/team-agent-settings.ts index 806cc84f9..facc08567 100644 --- a/packages/server/src/services/team-agent-settings.ts +++ b/packages/server/src/services/team-agent-settings.ts @@ -1,4 +1,6 @@ import { + GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS, + githubPermissionSatisfies, ORG_SETTINGS_NAMESPACES, type OrgGithubFeaturesOutput, type OrgGithubFeaturesStorage, @@ -74,7 +76,12 @@ async function readContextReviewerAgentUuid(db: Database, organizationId: string function taskReplyInstallationBlocker(installation: InstallationRow | null): SetupBlocker | null { if (!installation) return null; if (installation.suspendedAt) return blocker("github_app_suspended", "manage_github_installation"); - if (installation.permissions.issues !== "write" || installation.permissions.pull_requests !== "write") { + // Same requirement set the Settings → GitHub readout renders, so an admin is + // never shown a healthy connection while this gate refuses the assignment. + const unsatisfied = Object.entries(GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS).some( + ([permission, level]) => !githubPermissionSatisfies(installation.permissions[permission], level), + ); + if (unsatisfied) { return blocker("github_app_task_reply_permission_required", "manage_github_installation"); } return null; diff --git a/packages/shared/src/__tests__/github-task-reply-required-permissions.test.ts b/packages/shared/src/__tests__/github-task-reply-required-permissions.test.ts new file mode 100644 index 000000000..4f402fa99 --- /dev/null +++ b/packages/shared/src/__tests__/github-task-reply-required-permissions.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import { GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS, githubPermissionSatisfies } from "../schemas/github-app.js"; + +describe("GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS", () => { + it("is the set the task-reply gate enforces", () => { + // Both the server's `taskReplyInstallationBlocker` and the Settings → + // GitHub readout derive from this. Changing it changes what an admin is + // told AND what the server will publish, so it is pinned here. + expect(GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS).toEqual({ issues: "write", pull_requests: "write" }); + }); +}); + +describe("githubPermissionSatisfies", () => { + it("accepts an exact match", () => { + expect(githubPermissionSatisfies("write", "write")).toBe(true); + expect(githubPermissionSatisfies("read", "read")).toBe(true); + }); + + it("rejects a weaker grant", () => { + expect(githubPermissionSatisfies("read", "write")).toBe(false); + expect(githubPermissionSatisfies("write", "admin")).toBe(false); + }); + + it("does NOT rank levels — a stronger grant is not a substitute", () => { + // Every other installation-permission check in the server compares levels + // exactly. Ranking here would let this readout and the task-agent gate + // accept a grant that `github-audience` / the publishers then reject. + expect(githubPermissionSatisfies("admin", "write")).toBe(false); + expect(githubPermissionSatisfies("write", "read")).toBe(false); + }); + + it("treats an absent permission as unsatisfied", () => { + expect(githubPermissionSatisfies(undefined, "read")).toBe(false); + }); +}); diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 7accac3b3..1e49645a7 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -781,6 +781,7 @@ export { GITHUB_ACCOUNT_TYPES, GITHUB_APP_CONNECT_STATUSES, GITHUB_PERMISSION_LEVELS, + GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS, type GithubAccountType, type GithubAppConnectBody, type GithubAppConnectPanelInstallation, @@ -801,6 +802,7 @@ export { githubAppInstallationPermissionsSchema, githubAppUserTokenMetadataSchema, githubPermissionLevelSchema, + githubPermissionSatisfies, } from "./schemas/github-app.js"; export { GITHUB_TASK_REPLY_BODY_MAX_BYTES, diff --git a/packages/shared/src/schemas/github-app.ts b/packages/shared/src/schemas/github-app.ts index 771fbb6fc..f90d9a3ef 100644 --- a/packages/shared/src/schemas/github-app.ts +++ b/packages/shared/src/schemas/github-app.ts @@ -51,6 +51,54 @@ export type GithubPermissionLevel = z.infer; export const githubAppInstallationPermissionsSchema = z.record(z.string(), githubPermissionLevelSchema); export type GithubAppInstallationPermissions = z.infer; +/** + * The permission levels an installation must grant before the GitHub Task + * Agent can reply automatically on Issues and pull requests — + * `permission name -> required level`. + * + * Scoped to that one capability, not to the installation as a whole. Other + * First Tree capabilities gate on their own permissions, events, and + * repository coverage (Context Reviewer, the setup-capability probe, …), so + * this set answers "can automatic replies work here", never "is this + * installation generally usable". + * + * Within that scope it is the one definition: the server gates the GitHub Task + * Agent on it (`taskReplyInstallationBlocker`) and the Settings → GitHub + * readout renders it, so an admin is never told replies are fine while the + * server refuses to publish (or the reverse). + * + * `metadata: read` is deliberately absent: GitHub grants it to every App + * install and it cannot be withheld, so listing it would only ever render a + * tautological ✓. The reply publisher still requests it when minting a scoped + * token — that is a token scope, not a gate. + */ +export const GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS = { + issues: "write", + pull_requests: "write", +} as const satisfies Record; + +/** + * True when `granted` meets a required level. + * + * Deliberately an exact match, not a `read < write < admin` ranking. Every + * other installation-permission check in the server compares levels exactly + * (`github-audience.ts`, `github-task-reply-publisher.ts`, + * `context-reviewer-publisher.ts`, `setup-capabilities.ts`, …). A ranking here + * would let the Settings readout and the task-agent gate accept an `admin` + * grant that routing and publishing then reject — the exact contradiction this + * shared definition exists to prevent. GitHub only ever issues `read` / `write` + * for `issues` and `pull_requests`, so a ranking would buy nothing anyway. + * + * If the ranked semantics are ever wanted, change every call site together, + * not just this one. + */ +export function githubPermissionSatisfies( + granted: GithubPermissionLevel | undefined, + required: GithubPermissionLevel, +): boolean { + return granted === required; +} + /** * Subscribed event-name list, e.g. `["issues", "pull_request", "push"]`. * Free-form for the same forward-compat reason as `permissions`. diff --git a/packages/web/DESIGN.md b/packages/web/DESIGN.md index dac19f7ee..eb3269723 100644 --- a/packages/web/DESIGN.md +++ b/packages/web/DESIGN.md @@ -308,8 +308,8 @@ const buttonVariants = cva("…base classes…", { **Inventory** (representative): - **Actions / inputs:** Button, Input, Label, SegmentedControl, FilterPill, OptionCard, Command (cmdk), RowActionsMenu -- **Containers / layout:** Card, Panel, Section, Tile, SettingsField, PageHeader, - SectionHeader, FlatSectionHeader, TabBar +- **Containers / layout:** Card, Panel, Section, SettingRow, Tile, SettingsField, + PageHeader, SectionHeader, FlatSectionHeader, TabBar - **Data:** Table, DenseTable, Breadcrumb, Markdown - **Status / presence:** Badge, DenseBadge, StateChip, StateDot, StatusGlyph, PresenceChip, AgentStatusChip diff --git a/packages/web/src/components/ui/__tests__/setting-row.test.tsx b/packages/web/src/components/ui/__tests__/setting-row.test.tsx new file mode 100644 index 000000000..2ea838c94 --- /dev/null +++ b/packages/web/src/components/ui/__tests__/setting-row.test.tsx @@ -0,0 +1,112 @@ +// @vitest-environment happy-dom + +import { act, type ReactElement } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { SettingRow } from "../setting-row.js"; + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +async function renderDom(element: ReactElement): Promise<{ container: HTMLElement; root: Root }> { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + await act(async () => { + root.render(element); + }); + return { container, root }; +} + +beforeEach(() => { + document.body.innerHTML = ""; +}); + +afterEach(() => { + document.body.innerHTML = ""; +}); + +describe("SettingRow", () => { + it("renders name, effect, right-aligned control, and full-width extras", async () => { + const { container, root } = await renderDom( + Change} + > + Blocker copy + , + ); + + expect(container.textContent).toContain("GitHub Task Agent"); + expect(container.textContent).toContain("Dev Agent One handles Issue activity."); + const control = container.querySelector("button"); + expect(control?.textContent).toBe("Change"); + // The control belongs to the right-hand cluster, not the text column. + expect(control?.closest("[data-setting-row]")).not.toBeNull(); + expect(control?.parentElement?.className).toContain("sm:justify-end"); + // Extras render below the row line, outside the control cluster. + const extra = container.querySelector('[data-testid="extra"]'); + expect(extra?.closest("[data-setting-row]")).not.toBeNull(); + expect(extra?.parentElement?.contains(control)).toBe(false); + + await act(async () => root.unmount()); + }); + + it("hangs extras off the row's text column when it has a glyph", async () => { + const { container, root } = await renderDom( + } title="GitHub App"> + Connection details + , + ); + + // Without this the block resets to the container edge while the title sits + // indented past the glyph, leaving two competing left edges. + const extraWrapper = container.querySelector('[data-testid="extra"]')?.parentElement; + expect(extraWrapper?.style.paddingLeft).toBe("var(--setting-row-indent)"); + + await act(async () => root.unmount()); + }); + + it("leaves extras flush when there is no glyph to align to", async () => { + const { container, root } = await renderDom( + + Detail + , + ); + + const extraWrapper = container.querySelector('[data-testid="extra"]')?.parentElement; + expect(extraWrapper?.style.paddingLeft).toBe(""); + + await act(async () => root.unmount()); + }); + + it("draws no chrome of its own — the enclosing Section owns the rule", async () => { + const { container, root } = await renderDom(); + + const row = container.querySelector("[data-setting-row]"); + expect(row).not.toBeNull(); + expect(row?.style.border).toBe(""); + expect(row?.style.borderRadius).toBe(""); + expect(row?.style.background).toBe(""); + + await act(async () => root.unmount()); + }); + + it("spreads call-site hooks onto the row element", async () => { + const { container, root } = await renderDom(); + + const row = container.querySelector('[data-github-task-agent-controls="admin"]'); + expect(row?.getAttribute("data-setting-row")).toBe("true"); + + await act(async () => root.unmount()); + }); + + it("omits the description and control slots when not supplied", async () => { + const { container, root } = await renderDom(); + + expect(container.querySelector("p")).toBeNull(); + expect(container.querySelector(".sm\\:justify-end")).toBeNull(); + + await act(async () => root.unmount()); + }); +}); diff --git a/packages/web/src/components/ui/setting-row.tsx b/packages/web/src/components/ui/setting-row.tsx new file mode 100644 index 000000000..8a3c224e1 --- /dev/null +++ b/packages/web/src/components/ui/setting-row.tsx @@ -0,0 +1,103 @@ +import type { HTMLAttributes, ReactNode } from "react"; +import { cn } from "../../lib/utils.js"; + +/** + * One configurable thing inside a Settings `Section`: an optional leading + * glyph, the name of the setting, a one-line explanation of what it does, and + * the control that changes it — the control right-aligned on the same line. + * + * Why a row instead of the previous stack (label → description → full-width + * control): a settings section is scanned, not read. Keeping "what it is" on + * the left and "change it" on the right lets the eye run down a single control + * column, and stops a narrow `Select` from stretching across the whole page. + * + * The row draws no border, background, or radius of its own. The enclosing + * `Section` owns the single rule above the block, and card chrome stays + * reserved for genuinely selectable surfaces (DESIGN.md Pillar 5) — the leading + * glyph gets a small sunken tile so the row reads as an object, but the row + * content itself is never boxed. + * + * Extra props (`id`, `data-*`, `aria-*`) spread onto the row element so a call + * site can keep its own test/scroll hooks on the outermost node. + */ +export function SettingRow({ + icon, + title, + description, + control, + children, + className, + style, + ...rest +}: { + /** Leading line glyph (lucide, `h-4 w-4`), rendered in a sunken tile. */ + icon?: ReactNode; + /** What this row configures. */ + title: ReactNode; + /** One line on what the setting does / its current effect. */ + description?: ReactNode; + /** Right-aligned control — Switch, Select, Button cluster, or a status line. */ + control?: ReactNode; + /** Full-width content under the row: disclosures, blockers, inline errors. */ + children?: ReactNode; +} & Omit, "title" | "children">): ReactNode { + return ( +
+ {/* Stacked on phones so a wide control never overflows; one line with the + control right-aligned from sm up. */} +
+
+ {icon ? ( + + {icon} + + ) : null} +
+
+ {title} +
+ {description ? ( +

+ {description} +

+ ) : null} +
+
+ {control ? ( +
+ {control} +
+ ) : null} +
+ + {/* Extras hang off the row's text column, not the section edge. Without + this they reset to the container's left margin while the row's own + title sits indented past the glyph, so an expanded block reads as a + detached slab with a second, further-left edge. */} + {children ?
{children}
: null} +
+ ); +} + +/** + * Distance from the row's left edge to where its title starts — the glyph tile + * plus the gap after it, or nothing when the row has no glyph. + */ +function textColumnOffset(icon: ReactNode): string | undefined { + return icon ? "var(--setting-row-indent)" : undefined; +} diff --git a/packages/web/src/index.css b/packages/web/src/index.css index ecca60b1b..dffb92db7 100644 --- a/packages/web/src/index.css +++ b/packages/web/src/index.css @@ -554,6 +554,11 @@ video[data-onboarding-orientation-video]::cue { /* Shared label column for ConfigRow / IdentityField / DangerActionRow grids. */ --agent-detail-label-col: 8.25rem; + /* SettingRow's text column: the glyph tile (--sp-7) plus the gap after it + (--sp-2_5). Anything a row renders below its first line indents by this so + it hangs off the title instead of resetting to the section edge. */ + --setting-row-indent: 2.375rem; /* 38px */ + /* Default color-scheme so native form controls, scrollbars, and the canvas match the light theme (.dark flips this to dark below). */ color-scheme: light; diff --git a/packages/web/src/pages/__tests__/github-app-installation-panel-dom.test.tsx b/packages/web/src/pages/__tests__/github-app-installation-panel-dom.test.tsx index 246de309f..d6e3ad29b 100644 --- a/packages/web/src/pages/__tests__/github-app-installation-panel-dom.test.tsx +++ b/packages/web/src/pages/__tests__/github-app-installation-panel-dom.test.tsx @@ -174,6 +174,8 @@ describe("GithubAppInstallationPanel", () => { const { container, root } = await renderDom(); await waitForText(container, "Connected to"); + // The connection reads as one row: what it is, then who it's bound to. + expect(container.textContent).toContain("GitHub App"); // GitHub accounts render as the full github.com path so a GitHub org is // never confusable with a First Tree team name. expect(container.textContent).toContain("github.com/octocat"); @@ -190,16 +192,15 @@ describe("GithubAppInstallationPanel", () => { // the admin opens it. const detailsToggle = buttonByText(container, "Connection details"); expect(detailsToggle?.getAttribute("aria-expanded")).toBe("false"); - expect(container.textContent).not.toContain("contents:"); - expect(container.textContent).not.toContain(`Installation ${"#"}123`); + expect(container.textContent).not.toContain("Also granted"); await click(detailsToggle); expect(detailsToggle?.getAttribute("aria-expanded")).toBe("true"); - expect(container.textContent).toContain("contents:"); - expect(container.textContent).toContain("issues:"); - expect(container.textContent).toContain("pull_request"); - expect(container.textContent).toContain(`Installation ${"#"}123`); + expect(container.textContent).toContain("Required for automatic replies"); + expect(container.textContent).toContain("Also granted"); + expect(container.textContent).toContain("Contents"); + expect(container.textContent).toContain("Issues"); await act(async () => root.unmount()); }); diff --git a/packages/web/src/pages/__tests__/github-connection-details-dom.test.tsx b/packages/web/src/pages/__tests__/github-connection-details-dom.test.tsx new file mode 100644 index 000000000..a24310520 --- /dev/null +++ b/packages/web/src/pages/__tests__/github-connection-details-dom.test.tsx @@ -0,0 +1,196 @@ +// @vitest-environment happy-dom + +import type { GithubAppInstallationOutput } from "@first-tree/shared"; +import { act, type ReactElement } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { GithubConnectionDetails } from "../github-connection-details.js"; + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +function installation(overrides: Partial = {}): GithubAppInstallationOutput { + return { + installationId: 123, + accountType: "Organization", + accountLogin: "acme", + accountGithubId: 456, + permissions: { issues: "write", pull_requests: "write", metadata: "read", contents: "write" }, + events: ["issues", "issue_comment", "pull_request", "push", "member"], + suspended: false, + manageUrl: "https://github.com/organizations/acme/settings/installations/123", + createdAt: "2026-08-03T09:12:00.000Z", + updatedAt: "2026-08-06T16:41:00.000Z", + ...overrides, + }; +} + +async function renderDom(element: ReactElement): Promise<{ container: HTMLElement; root: Root }> { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + await act(async () => { + root.render(element); + }); + return { container, root }; +} + +function buttonByText(container: ParentNode, text: string): HTMLButtonElement | null { + return [...container.querySelectorAll("button")].find((button) => button.textContent?.includes(text)) ?? null; +} + +beforeEach(() => { + document.body.innerHTML = ""; +}); + +afterEach(() => { + document.body.innerHTML = ""; +}); + +describe("GithubConnectionDetails", () => { + it("keeps the detail behind a collapsed disclosure", async () => { + const { container, root } = await renderDom(); + + expect(buttonByText(container, "Connection details")?.getAttribute("aria-expanded")).toBe("false"); + expect(container.textContent).not.toContain("Required for automatic replies"); + expect(container.textContent).not.toContain("131952074"); + + await act(async () => root.unmount()); + }); + + it("scopes the requirement list to the capability it actually gates", async () => { + const { container, root } = await renderDom(); + + // `issues: write` / `pull_requests: write` only cover the GitHub Task + // Agent's automatic replies. Presenting them as First Tree's blanket + // install contract would tell an admin the installation is generally fine + // when only this one capability has been checked. + expect(container.textContent).toContain("Required for automatic replies"); + expect(container.textContent).not.toContain("Required by First Tree"); + expect(container.textContent).toContain("Issues"); + expect(container.textContent).toContain("Pull requests"); + // Everything else is secondary, not mixed into the requirement list. + expect(container.textContent).toContain("Also granted"); + expect(container.textContent).toContain("Contents"); + // Nothing is blocked, so no GitHub-side call to action appears. + expect(container.querySelector('a[href*="settings/installations"]')).toBeNull(); + + await act(async () => root.unmount()); + }); + + it("marks a shortfall, says what it costs, and points at the fix", async () => { + const data = installation({ permissions: { issues: "write", pull_requests: "read", metadata: "read" } }); + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("Agents can't post replies on pull requests."); + const grant = container.querySelector(`a[href="${data.manageUrl}"]`); + expect(grant?.textContent).toContain("Grant on GitHub"); + + await act(async () => root.unmount()); + }); + + it("surfaces the shortfall on the collapsed toggle so it isn't hidden", async () => { + const data = installation({ permissions: { issues: "write", pull_requests: "read" } }); + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("Pull requests access is missing"); + + await act(async () => root.unmount()); + }); + + it("counts multiple shortfalls rather than naming one of them", async () => { + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("2 required permissions are missing"); + + await act(async () => root.unmount()); + }); + + it("does not accept a level the rest of the server would reject", async () => { + // `github-audience` and the publishers compare exactly, so an `admin` grant + // must not read as satisfied here — that would promise delivery the routing + // path then refuses. + const data = installation({ permissions: { issues: "admin", pull_requests: "write" } }); + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("Agents can't post replies on issues."); + + await act(async () => root.unmount()); + }); + + it("withholds the GitHub call to action from members who can't act on it", async () => { + const data = installation({ permissions: { issues: "write", pull_requests: "read" } }); + const { container, root } = await renderDom(); + + // Members still read the same diagnosis... + expect(container.textContent).toContain("Agents can't post replies on pull requests."); + // ...without a button they have no standing to use. + expect(container.querySelector(`a[href="${data.manageUrl}"]`)).toBeNull(); + + await act(async () => root.unmount()); + }); + + it("names the events it consumes and calls out the ones it drops", async () => { + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("Issue comments"); + // `push` / `member` are subscribed but never normalized into activity — + // saying so is the answer when a webhook produced nothing. + expect(container.textContent).toContain("Subscribed but unused: push, member"); + + await act(async () => root.unmount()); + }); + + it("does not call handled lifecycle traffic unused", async () => { + // `installation` / `installation_repositories` never reach `buildRule`: + // the webhook route hands them to `handleInstallationLifecycle` first, and + // they are what keeps this very row and its repository coverage current. + // A normally configured installation subscribes to both, so filing them + // under "unused" reports a healthy install as misconfigured. + const data = installation({ events: ["issues", "installation", "installation_repositories", "push"] }); + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("Kept in sync from: Installation lifecycle · Repository access changes"); + expect(container.textContent).toContain("Subscribed but unused: push"); + expect(container.textContent).not.toContain("Subscribed but unused: installation"); + + await act(async () => root.unmount()); + }); + + it("renders the installation id and both timestamps the API already returns", async () => { + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("123"); + // "First seen", not "Connected": the row is written by the + // `installation.created` webhook while still unbound and survives a + // disconnect/rebind, so it never records when this team connected. + expect(container.textContent).toContain("First seen"); + expect(container.textContent).not.toContain("Connected"); + expect(container.textContent).toContain("Last updated"); + expect(container.textContent).toContain( + new Date("2026-08-03T09:12:00.000Z").toLocaleDateString(undefined, { + year: "numeric", + month: "short", + day: "numeric", + }), + ); + // House convention (inline-command.tsx / invite-link-panel.tsx): the copy + // control carries its state in the visible label, so the `failed` case + // `useCopyFeedback` returns can't render as an inert button. + expect(buttonByText(container, "Copy")).not.toBeNull(); + + await act(async () => root.unmount()); + }); + + it("degrades gracefully on permission and event names it has never seen", async () => { + const data = installation({ + permissions: { issues: "write", pull_requests: "write", dependabot_secrets: "read" }, + events: ["issues", "some_future_event"], + }); + const { container, root } = await renderDom(); + + expect(container.textContent).toContain("Dependabot secrets"); + expect(container.textContent).toContain("Subscribed but unused: some_future_event"); + + await act(async () => root.unmount()); + }); +}); diff --git a/packages/web/src/pages/__tests__/preview-pages-extra.test.tsx b/packages/web/src/pages/__tests__/preview-pages-extra.test.tsx index 8752a7a25..414e40c52 100644 --- a/packages/web/src/pages/__tests__/preview-pages-extra.test.tsx +++ b/packages/web/src/pages/__tests__/preview-pages-extra.test.tsx @@ -349,15 +349,21 @@ describe("extra preview pages", () => { expect(text(rendered.container)).toContain("waiting"); expect(text(rendered.container)).toContain("Loading"); expect(text(rendered.container)).toContain("Waiting for GitHub"); + // The two states that exist to review the shortfall readout. + expect(text(rendered.container)).toContain("a required scope is missing"); + expect(text(rendered.container)).toContain("Missing scope — collapsed"); + expect(text(rendered.container)).toContain("Agents can't post replies on pull requests."); + expect(text(rendered.container)).toContain("Pull requests access is missing"); const detailsButtons = [...rendered.container.querySelectorAll("button")].filter((button) => button.textContent?.includes("Connection details"), ); - expect(detailsButtons.length).toBe(3); + expect(detailsButtons.length).toBe(5); expect(detailsButtons[0]?.getAttribute("aria-expanded")).toBe("false"); await click(detailsButtons[0] ?? detailsButtons[1] ?? buttonByText(rendered.container, "Connection details")); expect(detailsButtons[0]?.getAttribute("aria-expanded")).toBe("true"); - expect(text(rendered.container)).toContain("Installation #131952074"); + expect(text(rendered.container)).toContain("131952074"); + expect(text(rendered.container)).toContain("Required for automatic replies"); await cleanupRendered(rendered); }); diff --git a/packages/web/src/pages/github-app-installation-panel.tsx b/packages/web/src/pages/github-app-installation-panel.tsx index e701ce6f6..f128b0445 100644 --- a/packages/web/src/pages/github-app-installation-panel.tsx +++ b/packages/web/src/pages/github-app-installation-panel.tsx @@ -1,6 +1,6 @@ import type { GithubAppConnectPanelInstallation, GithubAppInstallationOutput } from "@first-tree/shared"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { ArrowLeft, Building2, ChevronRight, ExternalLink, PauseCircle, User } from "lucide-react"; +import { ArrowLeft, Building2, ExternalLink, Github, PauseCircle, User } from "lucide-react"; import { type ReactNode, useEffect, useState } from "react"; import { ApiError } from "../api/client.js"; import { @@ -13,12 +13,14 @@ import { import { getAuthProviders, startProviderLink } from "../api/user-settings.js"; import { useAuth } from "../auth/auth-context.js"; import { Button } from "../components/ui/button.js"; +import { SettingRow } from "../components/ui/setting-row.js"; import { clearGithubAccountLinkReturn, rememberGithubAccountLinkReturn } from "../lib/github-account-link-return.js"; import { clearGithubInstallAttemptForOrganization, hasGithubInstallAttemptForOrganization, rememberGithubInstallAttempt, } from "../lib/github-install-attempt.js"; +import { GithubConnectionDetails } from "./github-connection-details.js"; /** * How often the open connect panel refreshes its installation list. Two @@ -108,7 +110,9 @@ export function GithubAppInstallationPanel({ /** * Unbound summary: the team has no GitHub connection yet, so the whole - * surface is one prominent entry point into the connect panel. + * surface is one prominent entry point into the connect panel. Same row shape + * as the connected state, so connecting doesn't reflow the section — only the + * status line and the right-hand control change. */ function NotConnectedSummary({ disabled, @@ -120,17 +124,18 @@ function NotConnectedSummary({ onOpenPanel: () => void; }) { return ( -
-

- This team isn't connected to GitHub yet. Connect a GitHub App installation to start receiving issues, pull - requests, and reviews as routed messages. -

- {!readOnly && ( - - )} -
+ } + title="GitHub App" + description="This team isn't connected to GitHub yet. Connect a GitHub App installation to start receiving issues, pull requests, and reviews as routed messages." + control={ + readOnly ? null : ( + + ) + } + /> ); } @@ -156,49 +161,43 @@ function InstalledState({ return ( // No borderTop of its own: the page's Section frame already draws the // rule above this block. -
+
{data.suspended && } -
-
- Connected to -
-
- - - {githubAccountPath(data.accountLogin)} - - - {data.accountType} + } + title="GitHub App" + // Who's connected reads as the row's status line rather than a separate + // labelled block — one glance answers "is this wired up, and to what". + description={ + + + + Connected to {githubAccountPath(data.accountLogin)} + + {data.accountType} -
+ } + control={ + readOnly ? null : ( + <> + + + + ) + } + > {/* Expandable details sit directly under the connected account they describe, not below the action buttons. */} -
- -
-
- - {!readOnly && ( -
- - -
- )} + +
); } @@ -723,89 +722,6 @@ function InstallationRow({ ); } -/** - * Collapsed-by-default disclosure for the developer-facing connection - * metadata: the granted permission scopes, the subscribed webhook events, - * and the installation id. Kept off the default view (most admins only need - * "who's connected" + Manage) but one click away for scope auditing. A plain - * `aria-expanded` button — there's no shared collapsible primitive in this app, - * and the controlled toggle keeps the chevron and the mounted content in - * lockstep. - */ -function ConnectionDetails({ data }: { data: GithubAppInstallationOutput }) { - const [open, setOpen] = useState(false); - const permissionEntries = Object.entries(data.permissions); - const regionId = "github-connection-details"; - - return ( -
- - - {open && ( -
- {permissionEntries.length > 0 && ( -
-
- Permissions granted -
-
    - {permissionEntries.map(([key, value]) => ( -
  • - {key}: {value} -
    • - ))} -
-
- )} - - {data.events.length > 0 && ( -
-
- Subscribed events -
-
- {data.events.join(", ")} -
-
- )} - - - Installation #{data.installationId} - -
- )} -
- ); -} - function SuspendedBanner() { return (
= { + actions: "Actions", + administration: "Administration", + checks: "Checks", + contents: "Contents", + discussions: "Discussions", + issues: "Issues", + members: "Members", + metadata: "Metadata", + organization_administration: "Organization administration", + pull_requests: "Pull requests", + workflows: "Workflows", +}; + +/** + * Webhook events First Tree turns into agent activity — mirrors the `buildRule` + * switch in the server's `github-normalize.ts`. These are the ones that can + * produce a message, which is what you're checking when an event seems to have + * gone nowhere. + */ +const ACTIVITY_EVENT_LABELS: Record = { + commit_comment: "Commit comments", + discussion: "Discussions", + discussion_comment: "Discussion comments", + issue_comment: "Issue comments", + issues: "Issues", + pull_request: "Pull requests", + pull_request_review: "PR reviews", + pull_request_review_comment: "PR review comments", +}; + +/** + * Events the webhook route consumes before it ever reaches `buildRule` — they + * create, refresh, suspend/resume and delete the installation record and its + * repository coverage (`api/webhooks/github-app.ts`, `handleInstallationLifecycle`). + * + * Classified apart from both lists on purpose: they produce no agent activity, + * so they can't sit under the activity heading, but they are very much handled, + * so grouping them with the dropped subscriptions would tell an admin their + * normally configured installation is misconfigured. + */ +const LIFECYCLE_EVENT_LABELS: Record = { + installation: "Installation lifecycle", + installation_repositories: "Repository access changes", +}; + +/** What the team loses while a required permission is missing. */ +const PERMISSION_SHORTFALL: Record = { + issues: "Agents can't post replies on issues.", + pull_requests: "Agents can't post replies on pull requests.", +}; + +function humanize(value: string): string { + const spaced = value.replace(/_/g, " "); + return spaced.charAt(0).toUpperCase() + spaced.slice(1); +} + +function permissionLabel(name: string): string { + return PERMISSION_LABELS[name] ?? humanize(name); +} + +function shortfallCopy(name: string, required: GithubPermissionLevel): string { + return PERMISSION_SHORTFALL[name] ?? `First Tree needs ${required} access here.`; +} + +type RequiredPermission = { + name: string; + required: GithubPermissionLevel; + granted: GithubPermissionLevel | undefined; + satisfied: boolean; +}; + +/** Collapsed-state summary: name the single gap, or count several. */ +function blockedSummary(blocked: RequiredPermission[]): string { + const [only] = blocked; + return blocked.length === 1 && only + ? `${permissionLabel(only.name)} access is missing` + : `${blocked.length} required permissions are missing`; +} + +function readRequiredPermissions(permissions: GithubAppInstallationOutput["permissions"]): RequiredPermission[] { + return Object.entries(GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS).map(([name, required]) => ({ + name, + required, + granted: permissions[name], + satisfied: githubPermissionSatisfies(permissions[name], required), + })); +} + +/** + * Collapsed-by-default disclosure for the connection's developer-facing + * detail. It used to transcribe GitHub's `permissions` / `events` blobs + * verbatim, which left the reader to diff them against a requirement they'd + * have to already know. It now answers the question they opened it with — + * "can this installation do the thing I'm waiting on?" — before falling back + * to the raw grants: + * + * - **Required for automatic replies** — one row per + * `GITHUB_TASK_REPLY_REQUIRED_PERMISSIONS` entry (the same set the server's + * task-reply gate reads), each marked ready or blocked. Scoped to that + * capability, not a verdict on the installation: other capabilities gate on + * their own permissions, events, and repository coverage. + * - **Also granted** — everything else, secondary. + * - **Events** — the subscriptions that produce agent activity, named in + * prose, with lifecycle traffic and genuinely dropped subscriptions each + * called out separately instead of blended together. + * - **Installation** — id (copyable), and the two timestamps the API already + * returns, which are the first thing you want when reconstructing "when + * did this change". + * + * A shortfall also marks the collapsed toggle, so the one state worth acting + * on isn't hidden behind a closed disclosure. + * + * A plain `aria-expanded` button — there's no shared collapsible primitive in + * this app, and the controlled toggle keeps the chevron and the mounted + * content in lockstep. + */ +export function GithubConnectionDetails({ + data, + readOnly = false, + defaultOpen = false, +}: { + data: GithubAppInstallationOutput; + /** Members read the same facts but get no GitHub-side call to action. */ + readOnly?: boolean; + /** The DEV `/preview/settings-github` gallery renders the expanded state directly. */ + defaultOpen?: boolean; +}) { + const [open, setOpen] = useState(defaultOpen); + // Per instance, not a module constant: the DEV gallery renders several of + // these on one page, and a shared literal would emit duplicate ids and an + // ambiguous `aria-controls`. + const regionId = useId(); + const required = readRequiredPermissions(data.permissions); + const blocked = required.filter((permission) => !permission.satisfied); + const requiredNames = new Set(required.map((permission) => permission.name)); + const alsoGranted = Object.entries(data.permissions) + .filter(([name]) => !requiredNames.has(name)) + .sort(([a], [b]) => permissionLabel(a).localeCompare(permissionLabel(b))); + const activity = data.events.filter((event) => event in ACTIVITY_EVENT_LABELS); + const lifecycle = data.events.filter((event) => event in LIFECYCLE_EVENT_LABELS); + const ignored = data.events.filter( + (event) => !(event in ACTIVITY_EVENT_LABELS) && !(event in LIFECYCLE_EVENT_LABELS), + ); + + return ( +
+
+ {/* `-ml-3` cancels the button's own `px-3` so the chevron lands on this + block's left edge instead of floating one padding step inside it. */} + + {/* Surface the one state worth acting on even while collapsed. */} + {blocked.length > 0 && !open ? ( + + + {blockedSummary(blocked)} + + ) : null} +
+ + {open && ( +
+ +
+ {required.map((permission) => ( + + ))} +
+ + + {alsoGranted.length > 0 && ( + +

+ {alsoGranted.map(([name, level], index) => ( + + {index > 0 ? " · " : ""} + {permissionLabel(name)} {level} + + ))} +

+ + )} + + +

+ {activity.length > 0 + ? activity.map((event) => ACTIVITY_EVENT_LABELS[event]).join(" · ") + : "None of this installation's subscriptions produce First Tree activity."} +

+ {lifecycle.length > 0 && ( + // Handled, just not by the activity path — say so, or a normally + // configured installation reads as carrying dead subscriptions. +

+ Kept in sync from: {lifecycle.map((event) => LIFECYCLE_EVENT_LABELS[event]).join(" · ")} +

+ )} + {ignored.length > 0 && ( + // Named rather than hidden: "you subscribed to it, we drop it" is + // the answer when someone is hunting a webhook that changed nothing. +

+ Subscribed but unused: {ignored.join(", ")} +

+ )} + + + +
+ + + {data.installationId} + + + + {/* "First seen", not "Connected": this is when First Tree first + stored the installation. The row is created by the + `installation.created` webhook while still unbound, and it + survives a disconnect/rebind — so it is not the moment this + team connected. Labelling it that way would put a wrong date + into exactly the timeline this block exists to reconstruct. */} + {formatDate(data.createdAt)} + {formatDate(data.updatedAt)} +
+ +
+ )} +
+ ); +} + +function RequiredPermissionRow({ + permission, + manageUrl, + readOnly, +}: { + permission: RequiredPermission; + manageUrl: string; + readOnly: boolean; +}) { + // Blocked, not "needs you": the fix lives on GitHub's side, and a First Tree + // admin role alone doesn't establish they can grant it there (DESIGN.md §3). + const Glyph = permission.satisfied ? CircleCheck : CircleAlert; + const color = permission.satisfied ? "var(--success)" : "var(--state-blocked)"; + return ( +
+ + + + {permissionLabel(permission.name)} + + + {permission.granted ?? "not granted"} + + {!permission.satisfied && ( + + {shortfallCopy(permission.name, permission.required)} + + )} + + {!permission.satisfied && !readOnly && ( + + )} +
+ ); +} + +/** + * Same shape as the other copy affordances in this app (`inline-command.tsx`, + * `invite-link-panel.tsx`): the visible label carries the state, including the + * `failed` one `useCopyFeedback` returns in a non-secure context or when the + * clipboard permission is denied. An icon-only button that ignores `failed` + * looks inert exactly when the copy didn't happen. + */ +function CopyInstallationId({ installationId }: { installationId: number }) { + const { status, copy } = useCopyFeedback(); + return ( + + ); +} + +function DetailBlock({ label, children }: { label: string; children: ReactNode }) { + return ( +
+
+ {label} +
+ {children} +
+ ); +} + +function DetailField({ term, children }: { term: string; children: ReactNode }) { + return ( + <> +
+ {term} +
+
+ {children} +
+ + ); +} + +/** + * Absolute dates, not "3 days ago": this block is read while reconstructing a + * timeline against GitHub's own audit log, where a relative age has to be + * converted back before it's usable. Named month over an all-numeric date so + * the day/month order can't be misread across locales. + */ +function formatDate(iso: string): string { + const parsed = new Date(iso); + if (Number.isNaN(parsed.getTime())) return iso; + return parsed.toLocaleDateString(undefined, { year: "numeric", month: "short", day: "numeric" }); +} diff --git a/packages/web/src/pages/settings-github-preview.tsx b/packages/web/src/pages/settings-github-preview.tsx index aca14f440..dfdf721cb 100644 --- a/packages/web/src/pages/settings-github-preview.tsx +++ b/packages/web/src/pages/settings-github-preview.tsx @@ -1,8 +1,12 @@ -import { Building2, ChevronRight, ExternalLink, PauseCircle, User } from "lucide-react"; +import type { GithubAppInstallationOutput } from "@first-tree/shared"; +import { ArrowRight, Bot, Building2, ExternalLink, FolderGit2, Github, PauseCircle, User } from "lucide-react"; import { useState } from "react"; +import { Button } from "../components/ui/button.js"; import { PageHeader } from "../components/ui/page-header.js"; import { Section } from "../components/ui/section.js"; import { Select } from "../components/ui/select.js"; +import { SettingRow } from "../components/ui/setting-row.js"; +import { GithubConnectionDetails } from "./github-connection-details.js"; /** * DEV-only visual review for Settings → GitHub (the connected GitHub App @@ -20,14 +24,17 @@ import { Select } from "../components/ui/select.js"; * - **Not installed** — the "Install on GitHub" CTA. * - **Loading** — the initial fetch state. * - * Connection and automatic handling are separate provider-owned sections. The - * markup here mirrors the real page closely enough to review their hierarchy - * without a live GitHub installation. + * Connection, automatic handling, and the Repositories hand-off are separate + * provider-owned sections, each a `SettingRow` (glyph + name + effect on the + * left, control right-aligned). The markup here mirrors the real page closely + * enough to review their hierarchy without a live GitHub installation. */ -const MOCK = { +const MOCK: GithubAppInstallationOutput = { + installationId: 131952074, + accountType: "Organization", accountLogin: "agent-team-foundation", - accountType: "Organization" as const, + accountGithubId: 987654, permissions: { issues: "write", members: "read", @@ -36,10 +43,31 @@ const MOCK = { contents: "write", pull_requests: "write", administration: "write", - } as Record, - events: ["issues", "issue_comment", "member", "pull_request", "pull_request_review", "push"], + }, + // `installation` / `installation_repositories` are in here because a real + // App subscribes to them — they are the traffic that keeps this installation + // row and its repository coverage current. The gallery has to show them so + // the three event classes (activity / lifecycle / dropped) can be reviewed. + events: [ + "installation", + "installation_repositories", + "issues", + "issue_comment", + "member", + "pull_request", + "pull_request_review", + "push", + ], + suspended: false, manageUrl: "https://github.com/organizations/agent-team-foundation/settings/installations/131952074", - installationId: 131952074, + createdAt: "2026-08-03T09:12:00.000Z", + updatedAt: "2026-08-06T16:41:00.000Z", +}; + +/** Same installation with a downgraded scope — the one state worth acting on. */ +const MOCK_MISSING_PERMISSION: GithubAppInstallationOutput = { + ...MOCK, + permissions: { ...MOCK.permissions, pull_requests: "read" }, }; const AccountIcon = MOCK.accountType === "Organization" ? Building2 : User; @@ -65,119 +93,51 @@ function SuspendedBanner() { ); } -function ConnectionDetails({ defaultOpen = false }: { defaultOpen?: boolean }) { - const [open, setOpen] = useState(defaultOpen); - const permissionEntries = Object.entries(MOCK.permissions); - const regionId = "github-connection-details"; - - return ( -
- - - {open && ( -
- {permissionEntries.length > 0 && ( -
-
- Permissions granted -
-
    - {permissionEntries.map(([key, value]) => ( -
  • - {key}: {value} -
    • - ))} -
-
- )} - {MOCK.events.length > 0 && ( -
-
- Subscribed events -
-
- {MOCK.events.join(", ")} -
-
- )} - - Installation #{MOCK.installationId} - -
- )} -
- ); -} - function AutomaticHandlingPreview() { const [agentUuid, setAgentUuid] = useState("dev-agent"); const agentName = agentUuid === "release-agent" ? "Release Agent" : "Dev Assistant"; return ( -
-
- - GitHub Task Agent - -
- {agentName} automatically handles Issue and pull request activity outside the Context Tree repository and - posts final replies as the First Tree GitHub App. + } + title="GitHub Task Agent" + description={`${agentName} automatically handles Issue and pull request activity outside the Context Tree repository and posts final replies as the First Tree GitHub App.`} + control={ +
+ + } + >
Context Tree activity uses Context Reviewer. These roles must use different Agents.
-
+ + ); +} + +function RepositoriesPointerPreview() { + return ( + } + title="Team code repositories" + description="Repository URLs and the agents that may clone them live in the Repositories tab — they apply to GitLab too, so they aren't tied to this connection." + control={ + + } + /> ); } @@ -194,64 +154,57 @@ function PageShell({ children }: { children: React.ReactNode }) { > +
+ +
); } -function InstalledCard({ suspended = false, detailsOpen = false }: { suspended?: boolean; detailsOpen?: boolean }) { +function InstalledCard({ + suspended = false, + detailsOpen = false, + data = MOCK, +}: { + suspended?: boolean; + detailsOpen?: boolean; + data?: GithubAppInstallationOutput; +}) { return ( -
+
{suspended && } -
-
- Connected to -
-
- - - {MOCK.accountLogin} + } + title="GitHub App" + description={ + + + + Connected to github.com/{data.accountLogin} + + {data.accountType} - - {MOCK.accountType} - -
-
- - + } + control={ + <> + + + + } + > + {/* The real component, not a copy: the gallery is only useful while it + can't drift from what ships. */} + +
); @@ -266,53 +219,28 @@ function InstalledCard({ suspended = false, detailsOpen = false }: { suspended?: function NotInstalledCard({ waiting = false }: { waiting?: boolean }) { return ( -
-

- Install the GitHub App on your personal account or organization to start receiving issues, pull requests, and - reviews as routed messages. -

- + } + title="GitHub App" + description="This team isn't connected to GitHub yet. Connect a GitHub App installation to start receiving issues, pull requests, and reviews as routed messages." + control={ + + } + > {waiting && ( -
+
Waiting for GitHub… - +
)} -
+ ); } @@ -372,9 +300,11 @@ export function SettingsGithubPreviewPage() { Settings → GitHub — shipped layout

- Lean default: who's connected + Manage on GitHub. Permissions, subscribed events, and the installation id sit - behind a collapsed "Connection details" disclosure (click to toggle). GitHub Task Agent lives in the adjacent - Automatic handling section instead of appearing as a standalone Setup capability. + Every section is one row: glyph + what it is + what it currently does on the left, the control that changes it + right-aligned. Permissions, subscribed events, and the installation id sit behind a collapsed "Connection + details" disclosure (click to toggle). GitHub Task Agent lives in the adjacent Automatic handling section + instead of appearing as a standalone Setup capability, and Repositories hands off to the provider-neutral + catalog rather than dead-ending here.

@@ -382,10 +312,27 @@ export function SettingsGithubPreviewPage() { - + + + + + + + + + diff --git a/packages/web/src/pages/settings/__tests__/github-page-dom.test.tsx b/packages/web/src/pages/settings/__tests__/github-page-dom.test.tsx index ae67fb8cb..77b21ff0c 100644 --- a/packages/web/src/pages/settings/__tests__/github-page-dom.test.tsx +++ b/packages/web/src/pages/settings/__tests__/github-page-dom.test.tsx @@ -344,6 +344,23 @@ describe("SettingsGithubPage — automatic handling", () => { await act(async () => root.unmount()); }); + it("hands off to the provider-neutral repository catalog below automatic handling", async () => { + const { SettingsGithubPage } = await import("../github.js"); + const { container, root } = await renderAt("/settings/integrations/github", ); + await waitForText(container, "Team code repositories"); + + const link = await waitForSelector( + container, + 'a[href="/settings/repositories#code-repositories"]', + ); + expect(link.textContent).toContain("Manage repositories"); + // The hand-off closes the page — it must not push automatic handling down. + const taskRouting = await waitForSelector(container, "#task-routing"); + expect(taskRouting.compareDocumentPosition(link) & Node.DOCUMENT_POSITION_FOLLOWING).not.toBe(0); + + await act(async () => root.unmount()); + }); + it("shows members the configured Agent without exposing assignment controls", async () => { authMock.value = { role: "member", diff --git a/packages/web/src/pages/settings/github-task-agent-controls.tsx b/packages/web/src/pages/settings/github-task-agent-controls.tsx index 01bb4ab53..0d499a7bf 100644 --- a/packages/web/src/pages/settings/github-task-agent-controls.tsx +++ b/packages/web/src/pages/settings/github-task-agent-controls.tsx @@ -1,6 +1,7 @@ import type { OrgGithubFeaturesOutput, SetupBlocker, TeamAgentCandidatesOutput } from "@first-tree/shared"; import { setupBlockerCodeSchema } from "@first-tree/shared"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { Bot } from "lucide-react"; import { useEffect, useState } from "react"; import { Link } from "react-router"; import { ApiError } from "../../api/client.js"; @@ -9,6 +10,7 @@ import { setupCapabilitiesQueryKey } from "../../api/setup-capabilities.js"; import { getTeamAgentCandidates, putTeamAgentAssignment } from "../../api/team-agent-settings.js"; import { useAuth } from "../../auth/auth-context.js"; import { Select } from "../../components/ui/select.js"; +import { SettingRow } from "../../components/ui/setting-row.js"; import { setupBlockerCopy } from "./setup-blocker-copy.js"; export function GithubTaskAgentControls({ @@ -62,24 +64,20 @@ export function GithubTaskAgentControls({ if (!isAdmin) { const projectedAgent = settingQuery.data?.teamAgent.agent ?? null; return ( -
- - GitHub Task Agent - - - {settingQuery.isLoading + icon={} + title="GitHub Task Agent" + description={ + settingQuery.isLoading ? "Loading configured Agent…" : settingQuery.error ? "First Tree could not load the configured Agent." : projectedAgent ? `${projectedAgent.displayName} automatically handles Issue and pull request activity outside the Context Tree repository and posts final replies as the First Tree GitHub App.` - : "Not configured. An admin can choose the Agent that automatically handles Issue and pull request activity outside the Context Tree repository and posts final replies as the First Tree GitHub App."} - -
+ : "Not configured. An admin can choose the Agent that automatically handles Issue and pull request activity outside the Context Tree repository and posts final replies as the First Tree GitHub App." + } + /> ); } @@ -110,32 +108,46 @@ export function GithubTaskAgentControls({ (item) => item.resolutionOwner === "admin" && item.actionKind === "manage_github_installation", ); const error = settingQuery.error ?? candidatesQuery.error ?? assignmentMutation.error; + const loading = settingQuery.isLoading || candidatesQuery.isLoading; + const assignable = !loading && candidates.length > 0; return ( -
} + title="GitHub Task Agent" + description={ + selectedLabel + ? `${selectedLabel} automatically handles Issue and pull request activity outside the Context Tree repository and posts final replies as the First Tree GitHub App.` + : "Choose the Agent that automatically handles Issue and pull request activity outside the Context Tree repository and posts final replies as the First Tree GitHub App." + } + control={ + loading ? ( + + Loading eligible Agents… + + ) : assignable ? ( + // The picker keeps a readable minimum instead of stretching the full + // page width — it is one control in a column of controls, not a form. +
+ { - const next = agentUuid || null; - if (next === selectedAgentUuid) return; - assignmentMutation.mutate(next); - }} - disabled={assignmentMutation.isPending} - options={options} - placeholder="Select an eligible Agent" - searchable={candidates.length > 6} - /> -
- Context Tree activity uses Context Reviewer. These roles must use different Agents. -
+ ) : null} + + {assignable ? ( +
+ Context Tree activity uses Context Reviewer. These roles must use different Agents.
- )} + ) : null} {error ? (
{teamAgentMutationError(error)}
) : null} -
+ ); } diff --git a/packages/web/src/pages/settings/github.tsx b/packages/web/src/pages/settings/github.tsx index 33d1c61bb..4d2309c8e 100644 --- a/packages/web/src/pages/settings/github.tsx +++ b/packages/web/src/pages/settings/github.tsx @@ -1,11 +1,12 @@ import { useQuery } from "@tanstack/react-query"; -import { ArrowRight, Check } from "lucide-react"; +import { ArrowRight, Check, FolderGit2 } from "lucide-react"; import { useEffect, useRef, useState } from "react"; import { Link, useLocation, useSearchParams } from "react-router"; import { getGithubAppInstallation } from "../../api/github-app.js"; import { useAuth } from "../../auth/auth-context.js"; import { Button } from "../../components/ui/button.js"; import { Section } from "../../components/ui/section.js"; +import { SettingRow } from "../../components/ui/setting-row.js"; import { clearGithubAccountLinkReturn, readGithubAccountLinkReturn } from "../../lib/github-account-link-return.js"; import { clearGithubInstallAttempt, readGithubInstallAttempt } from "../../lib/github-install-attempt.js"; import { GithubAppInstallationPanel } from "../github-app-installation-panel.js"; @@ -198,10 +199,37 @@ export function SettingsGithubPage() { +
); } +/** + * Connecting GitHub is the moment people go looking for "where do I add my + * repos" — but the code catalog is provider-neutral and lives on Settings → + * Repositories (this page used to host it under `#code-access`). Close the loop + * with an explicit hand-off instead of letting the page dead-end. + */ +function RepositoriesPointer() { + return ( +
+ } + title="Team code repositories" + description="Repository URLs and the agents that may clone them live in the Repositories tab — they apply to GitLab too, so they aren't tied to this connection." + control={ + + } + /> +
+ ); +} + /** * The Context round-trip return. Before the team is connected it's a quiet line * explaining why the user was sent here; once connected it becomes the explicit diff --git a/packages/web/src/pages/styleguide-preview.tsx b/packages/web/src/pages/styleguide-preview.tsx index f670ba57a..03e35486c 100644 --- a/packages/web/src/pages/styleguide-preview.tsx +++ b/packages/web/src/pages/styleguide-preview.tsx @@ -1,3 +1,4 @@ +import { Plug } from "lucide-react"; import { type ReactNode, useEffect, useState } from "react"; import { AgentStatusChip } from "../components/ui/agent-status-chip.js"; import { Badge } from "../components/ui/badge.js"; @@ -25,6 +26,7 @@ import { PresenceChip } from "../components/ui/presence-chip.js"; import { SectionHeader, UppercaseLabel } from "../components/ui/section-header.js"; import { SegmentedControl } from "../components/ui/segmented-control.js"; import { Select } from "../components/ui/select.js"; +import { SettingRow } from "../components/ui/setting-row.js"; import { StateChip } from "../components/ui/state-chip.js"; import { StateDot } from "../components/ui/state-dot.js"; import { StatusGlyph } from "../components/ui/status-glyph.js"; @@ -697,7 +699,7 @@ export function StyleguidePreviewPage() { {/* ─── Containers ────────────────────────────────────────────────── */} -
+
@@ -723,6 +725,31 @@ export function StyleguidePreviewPage() { + SettingRow +
+ {/* No chrome of its own — an enclosing Section owns the rule above. + Extras hang off the row's text column, not the container edge. */} + } + title="GitHub App" + description="Connected to github.com/acme" + control={ + + } + > + + Extras indent to the title, so the block reads as one column. + + + optional} + /> +
+ Tile