diff --git a/products/desktop/.claude/skills/merging-prs/SKILL.md b/products/desktop/.claude/skills/merging-prs/SKILL.md index 8959f52b7b53..cc8685b02de6 100644 --- a/products/desktop/.claude/skills/merging-prs/SKILL.md +++ b/products/desktop/.claude/skills/merging-prs/SKILL.md @@ -12,7 +12,7 @@ To merge, you enqueue the PR with a comment, then watch it until Trunk lands it. When a developer says "merge this PR", "merge it when it's ready", "land it", "ship it", or "babysit this PR", do the full loop below — enqueue **and** watch -to completion, reporting the outcome. See also [docs/merge-queue.md](../../../docs/merge-queue.md). +to completion, reporting the outcome. `` below is the PR number. Resolve the repo slug once if you need it: `REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner)`. @@ -29,8 +29,21 @@ gh pr view --json state,isDraft,mergeable,reviewDecision,statusCheckRollup - **Failing required checks** (`statusCheckRollup`) → the queue will just reject it. Report which checks are red and stop; fix them first. **Pending** checks are fine — the queue waits for them. +- **Not yet approved** (`reviewDecision` empty or `REVIEW_REQUIRED`) → fine, + enqueue anyway. See below. - **Merge conflicts** (`mergeable == "CONFLICTING"`) → report and stop; rebase first. +Enqueueing before approval is safe, and is the closest thing this repo has to +auto-merge. A submitted PR sits in Trunk's `Queued` state until GitHub's branch +protection on `main` is satisfied — one approving review, code-owner review, and +the required checks (`build`, `quality`, `unit-test`, `integration-test`, +`typecheck`) — and Trunk merges it once they land. Trunk is not a bypass actor on +those rules, so it cannot merge an unapproved or red PR. + +The catch: **pushing new commits drops the PR from the queue.** If review +feedback is likely, either wait for approval before enqueueing, or re-enqueue +with `/trunk merge` after each push. + ## 2. Enqueue ```bash @@ -68,7 +81,10 @@ sleep 60 - Watch the **check run + PR state**, not `gh pr checks --watch`: the queue runs CI on Trunk's own draft/`trunk-merge/**` branch, so this PR's own checks don't reflect the queue's testing. -- Stop at the timeout with a status summary rather than looping forever. +- If it's parked in `Queued` waiting on a human review, say so once and slow the + cadence to ~5 minutes. Keep watching — the merge still has to be reported. +- Stop at the timeout with a status summary rather than looping forever. If it + was still waiting on review, say that's why and offer to keep watching. ## 4. Handle failure diff --git a/products/desktop/MIGRATION.md b/products/desktop/MIGRATION.md index d60ca614cb56..6cbe1f5c144a 100644 --- a/products/desktop/MIGRATION.md +++ b/products/desktop/MIGRATION.md @@ -6,8 +6,8 @@ monorepo; anything else that differs from the source at the pinned SHA is drift be treated as a bug in the sync. - Source: https://github.com/PostHog/code -- Pinned SHA: `5ac5892a2f566b18125b2be89e8e11f17a7218e8` (main) -- Imported: 2026-07-20; resynced: 2026-07-30 +- Pinned SHA: `fc991d3eea2d1ac649e2502c0197b0fda9b19f58` (main) +- Imported: 2026-07-20; resynced: 2026-08-01 ## Resync protocol (for a human or an agent) @@ -45,14 +45,20 @@ The tree is a verbatim copy of the source at the pinned SHA except: `packages/ui/src/features/inbox/CLAUDE.md` renamed to `AGENTS.md` plus a symlink, and symlinks added in `packages/ui/src/features/{browser-tabs,canvas}/`. Upstream these to PostHog/code so resyncs do not reintroduce the violations. +- `docs/testing.md`: the "Storybook Visual Regression" CI paragraphs are replaced with a + note that the storybook CI was removed post-merge (see `POST-MIGRATION.md` step 6). The + source still documents its own storybook workflow; reapply on resync. - Local security patches (reapply on resync until the pin includes the upstream fix): `apps/code/src/main/utils/encryption.ts` passes `{ authTagLength: 16 }` to `createDecipheriv` (semgrep `gcm-no-tag-length`, ERROR). For the simple-git 3.36 RCE fix, - `packages/git/src/client.ts` opts into `unsafe.{allowUnsafeFsMonitor,allowUnsafeEditor, - allowUnsafePager}` (3.36's block-unsafe plugin otherwise rejects the hardcoded - core.fsmonitor perf flag and inherited GIT_EDITOR/PAGER), and `packages/git/src/queries.ts` - runs `git worktree list` through raw `execFile` instead of simple-git. All upstreamed to - PostHog/code (#4030). + `packages/git/package.json` bumps `simple-git` to `^3.36.0` (the source is on `^3.30.0`, + so the nested lockfile diverges there too), `packages/git/src/client.ts` opts into + `unsafe.{allowUnsafeFsMonitor,allowUnsafeEditor,allowUnsafePager}` (3.36's block-unsafe + plugin otherwise rejects the hardcoded core.fsmonitor perf flag and inherited + GIT_EDITOR/PAGER), and `packages/git/src/queries.ts` runs `git worktree list` through raw + `execFile` instead of simple-git. The upstream attempt (PostHog/code#4030) was closed + unmerged, so these stay local patches; restoring the four files from the previous + monorepo commit and refreshing the nested lockfile is the reapply. The nested workspace is intentional: `products/desktop/` keeps its own `pnpm-workspace.yaml`, lockfile, Biome config and Node 22, and is NOT in the root `pnpm-workspace.yaml` globs. diff --git a/products/desktop/apps/code/src/main/window.ts b/products/desktop/apps/code/src/main/window.ts index 62edd9f0ea57..48367c7c93b9 100644 --- a/products/desktop/apps/code/src/main/window.ts +++ b/products/desktop/apps/code/src/main/window.ts @@ -208,11 +208,14 @@ export function createWindow(): void { const platformWindowConfig = process.platform === "darwin" ? { - titleBarStyle: "hiddenInset" as const, + // "hidden", not "hiddenInset": hiddenInset keeps macOS's own inset and + // ignores trafficLightPosition's y, which parked the dots near the + // bottom of the bar. "hidden" honours the position we ask for. + titleBarStyle: "hidden" as const, // Centre the traffic lights vertically with the title bar's back/forward // buttons (40px bar, 24px buttons → centre at y=20; 12px dots → top at 14). // x mirrors y so the inset from the top and the left match. - trafficLightPosition: { x: 14, y: 14 }, + trafficLightPosition: { x: 14, y: 12 }, // Exposes the titlebar-area-* CSS env vars so the renderer can // clear the traffic lights exactly; their size varies by macOS // version (bigger on Tahoe), so it must not hardcode a width. diff --git a/products/desktop/apps/mobile/src/app/inbox/[...id].tsx b/products/desktop/apps/mobile/src/app/inbox/[...id].tsx index e13b504b6d20..b688222e8223 100644 --- a/products/desktop/apps/mobile/src/app/inbox/[...id].tsx +++ b/products/desktop/apps/mobile/src/app/inbox/[...id].tsx @@ -464,7 +464,7 @@ export default function ReportDetailScreen() { {/* Title */} - {report.title ?? "Untitled signal"} + {report.title ?? "Untitled report"} {/* Meta row */} @@ -642,7 +642,7 @@ export default function ReportDetailScreen() { setDismissOpen(false)} onDismissed={handleDismissed} /> diff --git a/products/desktop/apps/mobile/src/features/inbox/components/ArchivedReportList.tsx b/products/desktop/apps/mobile/src/features/inbox/components/ArchivedReportList.tsx index 72de97273729..03524e145dbc 100644 --- a/products/desktop/apps/mobile/src/features/inbox/components/ArchivedReportList.tsx +++ b/products/desktop/apps/mobile/src/features/inbox/components/ArchivedReportList.tsx @@ -55,7 +55,7 @@ const ArchivedRow = memo(function ArchivedRow({ numberOfLines={2} ellipsizeMode="tail" > - {report.title ?? "Untitled signal"} + {report.title ?? "Untitled report"} diff --git a/products/desktop/apps/mobile/src/features/inbox/components/ReportListRow.tsx b/products/desktop/apps/mobile/src/features/inbox/components/ReportListRow.tsx index 3b0508bb8354..e12cff57bfdd 100644 --- a/products/desktop/apps/mobile/src/features/inbox/components/ReportListRow.tsx +++ b/products/desktop/apps/mobile/src/features/inbox/components/ReportListRow.tsx @@ -70,7 +70,7 @@ function ReportListRowComponent({ report, onPress }: ReportListRowProps) { numberOfLines={2} ellipsizeMode="tail" > - {report.title ?? "Untitled signal"} + {report.title ?? "Untitled report"} diff --git a/products/desktop/apps/web/package.json b/products/desktop/apps/web/package.json index bf1eff1d7174..dfd6a082f23b 100644 --- a/products/desktop/apps/web/package.json +++ b/products/desktop/apps/web/package.json @@ -19,6 +19,7 @@ "@posthog/core": "workspace:*", "@posthog/di": "workspace:*", "@posthog/host-router": "workspace:*", + "@posthog/harness": "workspace:*", "@posthog/host-trpc": "workspace:*", "@posthog/platform": "workspace:*", "@posthog/shared": "workspace:*", diff --git a/products/desktop/apps/web/src/web-host-router.ts b/products/desktop/apps/web/src/web-host-router.ts index 07f148b42d10..fb2ac0a73a71 100644 --- a/products/desktop/apps/web/src/web-host-router.ts +++ b/products/desktop/apps/web/src/web-host-router.ts @@ -1,10 +1,10 @@ -import { fetchPosthogPiModelCatalog } from "@posthog/agent/pi/model-catalog"; import { getLlmGatewayUrl } from "@posthog/agent/posthog-api"; import type { AuthService } from "@posthog/core/auth/auth"; import { AUTH_SERVICE } from "@posthog/core/auth/auth.module"; import { TEAM_SKILLS_SERVICE } from "@posthog/core/skills/identifiers"; import type { TeamSkillsService } from "@posthog/core/skills/teamSkillsService"; import { resolveService } from "@posthog/di/container"; +import { fetchPosthogPiModelCatalog } from "@posthog/harness/extensions/posthog-provider/model-catalog"; import { analyticsRouter } from "@posthog/host-router/routers/analytics.router"; import { authRouter } from "@posthog/host-router/routers/auth.router"; import { canvasDataRouter } from "@posthog/host-router/routers/canvas-data.router"; diff --git a/products/desktop/docs/LOCAL-DEVELOPMENT.md b/products/desktop/docs/LOCAL-DEVELOPMENT.md index 41c375f2d061..0991eeaf6661 100644 --- a/products/desktop/docs/LOCAL-DEVELOPMENT.md +++ b/products/desktop/docs/LOCAL-DEVELOPMENT.md @@ -127,10 +127,59 @@ region you pick at login. ## Troubleshooting +### Feature flags never enabled (flag-gated UI missing) + +If flag-gated surfaces (e.g. the MCP gateway behind `mcp-gateway`) never show up +even though the flag is enabled in your PostHog project, check +`VITE_POSTHOG_API_HOST` in `.env`: it must include the scheme +(`http://localhost:8010`, not `localhost:8010`). posthog-js concatenates the +host into request URLs verbatim, so a scheme-less value produces URLs like +`localhost:8010/flags/…` that the browser rejects as an invalid protocol — +every flag fetch fails silently and `isFeatureEnabled` returns `undefined` for +everything (flags never loaded). Prefer `node scripts/use-local-posthog.mjs` +over hand-editing; it writes the correct form. + +To confirm what the running app sees, run in the renderer console (or via CDP): + +```js +posthog.config.api_host; // must start with http:// or https:// +posthog.isFeatureEnabled("mcp-gateway"); // undefined ⇒ flags never loaded +``` + +`.env` changes need a dev-server restart (`pnpm dev`) to take effect. + ### "Invalid client_id" error during OAuth The OAuth application in your local PostHog must have the client ID `DC5uRLVbGI02YQ82grxgnK6Qn12SXWpCqdPb60oZ`. Verify at http://localhost:8010/admin/posthog/oauthapplication/. +### "OAuth error: invalid_scope" + +PostHog Code requests the wildcard scope `*` (see `OAUTH_SCOPES` in +`packages/shared/src/oauth.ts`). PostHog's OAuth server only grants `*` at +`/authorize` when the OAuth application's **scope ceiling is empty** — this is +the grandfathering path for the PostHog Code client. If the application has any +explicit `scopes` or `optional_scopes` configured, the wildcard is rejected with +`invalid_scope`. + +Fix: clear the scope ceiling on your local OAuth application so it matches the +production app. Either edit it at +http://localhost:8010/admin/posthog/oauthapplication/ (empty the **Scopes** and +**Optional scopes** fields), or run in your PostHog repo: + +```bash +python manage.py shell -c " +from posthog.models.oauth import OAuthApplication +app = OAuthApplication.objects.get(client_id='DC5uRLVbGI02YQ82grxgnK6Qn12SXWpCqdPb60oZ') +app.scopes = [] +app.optional_scopes = [] +app.save() +print('cleared scope ceiling for', app.client_id) +" +``` + +Then retry login. (Do not add `*` to the ceiling — an explicit ceiling never +grants the wildcard, even if `*` is listed.) + ### "Redirect URI mismatch" Make sure the OAuth application's redirect URIs include `http://localhost:8237/callback` and `http://localhost:8239/callback`. Check for trailing slashes. diff --git a/products/desktop/packages/agent/package.json b/products/desktop/packages/agent/package.json index 0f7d72b8df9c..e159b71d0e2d 100644 --- a/products/desktop/packages/agent/package.json +++ b/products/desktop/packages/agent/package.json @@ -52,10 +52,6 @@ "types": "./dist/pi/types.d.ts", "import": "./dist/pi/types.js" }, - "./pi/model-catalog": { - "types": "./dist/pi/model-catalog.d.ts", - "import": "./dist/pi/model-catalog.js" - }, "./pr-url-detector": { "types": "./dist/pr-url-detector.d.ts", "import": "./dist/pr-url-detector.js" diff --git a/products/desktop/packages/agent/src/adapters/claude/session/options.test.ts b/products/desktop/packages/agent/src/adapters/claude/session/options.test.ts index f2a0b66c99d1..418b2c8d9e7b 100644 --- a/products/desktop/packages/agent/src/adapters/claude/session/options.test.ts +++ b/products/desktop/packages/agent/src/adapters/claude/session/options.test.ts @@ -376,7 +376,10 @@ describe("buildSessionOptions", () => { name: "omits the team_id header when POSTHOG_PROJECT_ID is unset", projectId: undefined, existingHeaders: undefined, - expected: "x-posthog-use-bedrock-fallback: true", + expected: [ + "x-posthog-property-$ai_session_id: test-session", + "x-posthog-use-bedrock-fallback: true", + ].join("\n"), }, { name: "forwards POSTHOG_PROJECT_ID as the team_id attribution header", @@ -384,6 +387,7 @@ describe("buildSessionOptions", () => { existingHeaders: undefined, expected: [ "x-posthog-property-team_id: 42", + "x-posthog-property-$ai_session_id: test-session", "x-posthog-use-bedrock-fallback: true", ].join("\n"), }, @@ -394,6 +398,7 @@ describe("buildSessionOptions", () => { expected: [ "x-posthog-property-task_id: task-abc", "x-posthog-property-team_id: 42", + "x-posthog-property-$ai_session_id: test-session", "x-posthog-use-bedrock-fallback: true", ].join("\n"), }, @@ -411,6 +416,101 @@ describe("buildSessionOptions", () => { expect(headers).toBe(expected); }); }); + + describe("gateway turn tracing env", () => { + const KEYS = [ + "CLAUDE_CODE_ENABLE_TELEMETRY", + "CLAUDE_CODE_ENHANCED_TELEMETRY_BETA", + "CLAUDE_CODE_PROPAGATE_TRACEPARENT", + "OTEL_TRACES_EXPORTER", + "OTEL_EXPORTER_OTLP_PROTOCOL", + "OTEL_EXPORTER_OTLP_ENDPOINT", + "TRACEPARENT", + "TRACESTATE", + ] as const; + const original: Partial> = {}; + + beforeEach(() => { + for (const key of KEYS) { + original[key] = process.env[key]; + delete process.env[key]; + } + }); + + afterEach(() => { + for (const key of KEYS) { + const value = original[key]; + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + const gatewayEnv = { + anthropicBaseUrl: "https://gateway.example", + anthropicAuthToken: "tok", + openaiBaseUrl: "https://gateway.example/v1", + openaiApiKey: "tok", + }; + + it("enables per-turn traceparent when routed through the gateway", () => { + const env = buildSessionOptions({ ...makeParams(), gatewayEnv }).env; + + expect(env?.CLAUDE_CODE_ENABLE_TELEMETRY).toBe("1"); + expect(env?.CLAUDE_CODE_ENHANCED_TELEMETRY_BETA).toBe("1"); + expect(env?.CLAUDE_CODE_PROPAGATE_TRACEPARENT).toBe("1"); + expect(env?.OTEL_TRACES_EXPORTER).toBe("otlp"); + expect(env?.OTEL_EXPORTER_OTLP_PROTOCOL).toBe("http/json"); + expect(env?.OTEL_EXPORTER_OTLP_ENDPOINT).toBe("http://127.0.0.1:9"); + }); + + it("honors a caller-supplied OTLP endpoint", () => { + process.env.OTEL_EXPORTER_OTLP_ENDPOINT = + "http://collector.internal:4318"; + + const env = buildSessionOptions({ ...makeParams(), gatewayEnv }).env; + + expect(env?.OTEL_EXPORTER_OTLP_ENDPOINT).toBe( + "http://collector.internal:4318", + ); + }); + + it("pins exporter and protocol so an inherited none can't disable tracing", () => { + process.env.OTEL_TRACES_EXPORTER = "none"; + process.env.OTEL_EXPORTER_OTLP_PROTOCOL = "grpc"; + + const env = buildSessionOptions({ ...makeParams(), gatewayEnv }).env; + + expect(env?.OTEL_TRACES_EXPORTER).toBe("otlp"); + expect(env?.OTEL_EXPORTER_OTLP_PROTOCOL).toBe("http/json"); + }); + + it("strips inherited TRACEPARENT so turns keep distinct trace ids", () => { + process.env.TRACEPARENT = + "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"; + process.env.TRACESTATE = "vendor=x"; + + const env = buildSessionOptions({ ...makeParams(), gatewayEnv }).env; + + expect(env?.TRACEPARENT).toBeUndefined(); + expect(env?.TRACESTATE).toBeUndefined(); + }); + + it("leaves BYOK sessions untouched", () => { + process.env.TRACEPARENT = + "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"; + + const env = buildSessionOptions(makeParams()).env; + + expect(env?.CLAUDE_CODE_ENABLE_TELEMETRY).toBeUndefined(); + expect(env?.CLAUDE_CODE_PROPAGATE_TRACEPARENT).toBeUndefined(); + expect(env?.TRACEPARENT).toBe( + "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01", + ); + }); + }); }); describe("buildSystemPrompt", () => { diff --git a/products/desktop/packages/agent/src/adapters/claude/session/options.ts b/products/desktop/packages/agent/src/adapters/claude/session/options.ts index a5e08ccc79f5..25c6743d2fe9 100644 --- a/products/desktop/packages/agent/src/adapters/claude/session/options.ts +++ b/products/desktop/packages/agent/src/adapters/claude/session/options.ts @@ -151,7 +151,10 @@ function buildMcpServers( }; } -function buildEnvironment(gateway?: GatewayEnv): Record { +function buildEnvironment( + gateway?: GatewayEnv, + sessionId?: string, +): Record { // Custom HTTP headers reach the model only through the Claude CLI subprocess, // which reads them from this env var (newline-delimited `name: value` lines) // — the SDK has no direct header option. We finalize them here, the single @@ -174,6 +177,11 @@ function buildEnvironment(gateway?: GatewayEnv): Record { if (projectId) { headerLines.push(buildGatewayPropertyHeaders({ team_id: projectId })); } + if (sessionId) { + headerLines.push( + buildGatewayPropertyHeaders({ $ai_session_id: sessionId }), + ); + } // Route to AWS Bedrock as a fallback when Anthropic returns 5xx headerLines.push("x-posthog-use-bedrock-fallback: true"); const customHeaders = headerLines.join("\n"); @@ -185,8 +193,31 @@ function buildEnvironment(gateway?: GatewayEnv): Record { // sessions that genuinely need MCP tools available on turn 1. const mcpNonblocking = process.env.MCP_CONNECTION_NONBLOCKING; - return { + // Every var is load-bearing (ablation-tested): the CLI stamps the per-turn + // traceparent only once its OTel tracer initializes, and the dead endpoint + // keeps the throwaway spans off any local collector. Exporter and protocol + // are pinned rather than inherited — an ambient OTEL_TRACES_EXPORTER=none or + // unknown protocol registers no tracer and silently drops the traceparent; + // the endpoint stays overridable for a real collector. + // Residual risk: a repo's .claude/settings.json `env` is applied over these + // inside the CLI and can redirect the endpoint or turn on content capture + // (OTEL_LOG_TOOL_CONTENT, …) — pre-existing settingSources exposure, not + // closable from here; hardening tracked separately. + const gatewayTracing: Record = gateway?.anthropicBaseUrl + ? { + CLAUDE_CODE_ENABLE_TELEMETRY: "1", + CLAUDE_CODE_ENHANCED_TELEMETRY_BETA: "1", + CLAUDE_CODE_PROPAGATE_TRACEPARENT: "1", + OTEL_TRACES_EXPORTER: "otlp", + OTEL_EXPORTER_OTLP_PROTOCOL: "http/json", + OTEL_EXPORTER_OTLP_ENDPOINT: + process.env.OTEL_EXPORTER_OTLP_ENDPOINT ?? "http://127.0.0.1:9", + } + : {}; + + const env: Record = { ...process.env, + ...gatewayTracing, // Explicit gateway values win over whatever happens to be in process.env. // This prevents concurrent Agent instances from clobbering each other's // gateway config when process.env was mutated globally. @@ -212,6 +243,13 @@ function buildEnvironment(gateway?: GatewayEnv): Record { }), ANTHROPIC_CUSTOM_HEADERS: customHeaders, }; + if (gateway?.anthropicBaseUrl) { + // The CLI parents every turn under an inherited ambient TRACEPARENT, + // collapsing the per-turn trace ids this block exists to produce. + delete env.TRACEPARENT; + delete env.TRACESTATE; + } + return env; } function buildHooks( @@ -473,7 +511,7 @@ export function buildSessionOptions(params: BuildOptionsParams): Options { params.mcpServers, loadUserClaudeJsonMcpServers(params.cwd, params.logger), ), - env: buildEnvironment(params.gatewayEnv), + env: buildEnvironment(params.gatewayEnv, params.sessionId), hooks: buildHooks( params.userProvidedOptions?.hooks, params.onModeChange, diff --git a/products/desktop/packages/agent/src/adapters/codex-app-server/spawn.test.ts b/products/desktop/packages/agent/src/adapters/codex-app-server/spawn.test.ts index 6794bef2dd60..a5784252d4f7 100644 --- a/products/desktop/packages/agent/src/adapters/codex-app-server/spawn.test.ts +++ b/products/desktop/packages/agent/src/adapters/codex-app-server/spawn.test.ts @@ -57,6 +57,18 @@ describe("buildAppServerArgs", () => { ); }); + it("quotes $-prefixed posthog property header keys in the TOML table", () => { + const args = buildAppServerArgs({ + binaryPath: "/bundle/codex", + apiBaseUrl: "https://gateway.example/v1", + httpHeaders: { "x-posthog-property-$ai_session_id": "task-123" }, + }); + + expect(args).toContain( + 'model_providers.posthog.http_headers={ "x-posthog-property-$ai_session_id" = "task-123" }', + ); + }); + it("omits http_headers when none are provided or the provider is unset", () => { const withoutHeaders = buildAppServerArgs({ binaryPath: "/bundle/codex", diff --git a/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-commit.test.ts b/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-commit.test.ts index 33378c3f1fa7..5502dacd3be7 100644 --- a/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-commit.test.ts +++ b/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-commit.test.ts @@ -117,6 +117,41 @@ describe("signed-commit tool handler", () => { }); }); + it("persists the branch when cwd uses an equivalent path representation", async () => { + await signedCommitTool.handler( + { + cwd: "/tmp/workspace/repos/posthog/code/.", + token: "ghs_x", + taskId: "task-1", + taskRunId: "run-1", + }, + { message: "chore: bump", cwd: "." }, + ); + + expect(reportTaskRunBranch).toHaveBeenCalledWith({ + taskId: "task-1", + taskRunId: "run-1", + branch: "posthog-code/feature", + }); + }); + + it("does not persist a branch created in a sibling repository", async () => { + await signedCommitTool.handler( + { + cwd: "/tmp/workspace/repos/posthog/code", + token: "ghs_x", + taskId: "task-1", + taskRunId: "run-1", + }, + { + message: "chore: bump", + cwd: "/tmp/workspace/repos/posthog/grafana-dashboards", + }, + ); + + expect(reportTaskRunBranch).not.toHaveBeenCalled(); + }); + it("returns the no-token error without invoking createSignedCommit", async () => { const savedGh = process.env.GH_TOKEN; const savedGithub = process.env.GITHUB_TOKEN; diff --git a/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-git-tool.ts b/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-git-tool.ts index 0b72c25875fc..cdeae219d0ec 100644 --- a/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-git-tool.ts +++ b/products/desktop/packages/agent/src/adapters/local-tools/tools/signed-git-tool.ts @@ -43,10 +43,14 @@ export function defineSignedGitTool(opts: { string, unknown >; - const cwd = argCwd ? path.resolve(ctx.cwd, argCwd) : ctx.cwd; + const taskRepositoryCwd = path.resolve(ctx.cwd); + const cwd = argCwd + ? path.resolve(taskRepositoryCwd, argCwd) + : taskRepositoryCwd; return opts.run( { cwd, + taskRepositoryCwd, token, taskId: ctx.taskId, taskRunId: ctx.taskRunId, diff --git a/products/desktop/packages/agent/src/adapters/signed-commit-shared.ts b/products/desktop/packages/agent/src/adapters/signed-commit-shared.ts index aa7e8523b4de..2cdd83eb7d24 100644 --- a/products/desktop/packages/agent/src/adapters/signed-commit-shared.ts +++ b/products/desktop/packages/agent/src/adapters/signed-commit-shared.ts @@ -135,7 +135,11 @@ export interface SignedCommitToolResult { [key: string]: unknown; } -export type SignedCommitToolCtx = SignedCommitCtx & { taskRunId?: string }; +export type SignedCommitToolCtx = SignedCommitCtx & { + taskRunId?: string; + /** The task repository cwd, before a tool-call `cwd` override is applied. */ + taskRepositoryCwd: string; +}; async function runSignedTool( toolName: string, @@ -176,11 +180,17 @@ export function runSignedCommitTool( SIGNED_COMMIT_TOOL_NAME, async (c, a: SignedCommitInput) => { const result = await createSignedCommit(c, a); - await reportTaskRunBranch({ - taskId: ctx.taskId, - taskRunId: ctx.taskRunId, - branch: result.branch, - }); + // TaskRun.branch is the branch that provisioning checks out in the task's + // repository on resume. A task can also commit to sibling repositories by + // passing `cwd`; persisting one of those branches here makes the next run + // try to clone the task repository at a branch that only exists elsewhere. + if (ctx.cwd === ctx.taskRepositoryCwd) { + await reportTaskRunBranch({ + taskId: ctx.taskId, + taskRunId: ctx.taskRunId, + branch: result.branch, + }); + } // The "commit hook": every pushed commit becomes a `commit` artefact on the signal // reports this task is associated with. Best-effort and awaited inside the tool's // try/catch-free success path — reportCommitArtefacts never throws, so a failed diff --git a/products/desktop/packages/agent/src/agent.ts b/products/desktop/packages/agent/src/agent.ts index 63b383084975..b7ea903d8d2c 100644 --- a/products/desktop/packages/agent/src/agent.ts +++ b/products/desktop/packages/agent/src/agent.ts @@ -14,6 +14,7 @@ import { import { PostHogAPIClient, type TaskRunUpdate } from "./posthog-api"; import { SessionLogWriter } from "./session-log-writer"; import type { AgentConfig, TaskExecutionOptions } from "./types"; +import { buildGatewayPropertyHeaderRecord } from "./utils/gateway"; import { Logger } from "./utils/logger"; export class Agent { @@ -148,6 +149,9 @@ export class Agent { model: sanitizedModel, reasoningEffort: options.reasoningEffort, developerInstructions: options.developerInstructions, + httpHeaders: taskId + ? buildGatewayPropertyHeaderRecord({ $ai_session_id: taskId }) + : undefined, additionalDirectories: options.additionalDirectories, } : undefined, diff --git a/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.test.ts b/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.test.ts index 4db19cb58ecc..f0f3e4d6b55f 100644 --- a/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.test.ts +++ b/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.test.ts @@ -162,11 +162,17 @@ describe("createPiConversationTranslator", () => { delayMs: 1000, }, ]); + const retriedMessage = assistant([{ type: "text", text: "Done" }]); expect( translator.translateEvent({ - type: "auto_retry_end", - success: true, - attempt: 1, + type: "message_update", + message: retriedMessage, + assistantMessageEvent: { + type: "text_delta", + contentIndex: 0, + delta: "Done", + partial: retriedMessage, + }, }), ).toEqual([ { @@ -175,7 +181,19 @@ describe("createPiConversationTranslator", () => { status: "retrying", isComplete: true, }, + { + type: "assistant_message_chunk", + timestamp: 10, + content: { type: "text", text: "Done" }, + }, ]); + expect( + translator.translateEvent({ + type: "auto_retry_end", + success: true, + attempt: 1, + }), + ).toEqual([]); }); it("renders terminal Pi runtime errors inline", () => { diff --git a/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.ts b/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.ts index 745145623de3..ad025153a0bd 100644 --- a/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.ts +++ b/products/desktop/packages/agent/src/pi/conversation/translatePiConversation.ts @@ -115,7 +115,25 @@ export function createPiConversationTranslator(): PiConversationTranslator { let latestRuntimeTimestamp = 0; let latestConversationTimestamp = 0; let pendingRuntimeError: AgentConversationEvent | undefined; + let retrying = false; let directBashSequence = 0; + + function completeRetry(timestamp: number): AgentConversationEvent[] { + if (!retrying) { + return []; + } + + retrying = false; + return [ + { + type: "runtime_status", + timestamp, + status: "retrying", + isComplete: true, + }, + ]; + } + let activeDirectBash: | { nextOutputBytes: number; @@ -257,6 +275,7 @@ export function createPiConversationTranslator(): PiConversationTranslator { if (update.type === "text_delta" && update.delta) { streamedAssistantTimestamps.add(event.message.timestamp); return [ + ...completeRetry(event.message.timestamp), { type: "assistant_message_chunk", timestamp: event.message.timestamp, @@ -268,6 +287,7 @@ export function createPiConversationTranslator(): PiConversationTranslator { if (update.type === "thinking_delta" && update.delta) { streamedAssistantTimestamps.add(event.message.timestamp); return [ + ...completeRetry(event.message.timestamp), { type: "assistant_thought_chunk", timestamp: event.message.timestamp, @@ -407,7 +427,11 @@ export function createPiConversationTranslator(): PiConversationTranslator { } if (event.type === "auto_retry_start") { + const completedEvents = completeRetry(latestConversationTimestamp); + retrying = true; + return [ + ...completedEvents, { type: "runtime_status", timestamp: latestConversationTimestamp, @@ -421,14 +445,9 @@ export function createPiConversationTranslator(): PiConversationTranslator { } if (event.type === "auto_retry_end") { - const events: AgentConversationEvent[] = [ - { - type: "runtime_status", - timestamp: latestConversationTimestamp, - status: "retrying", - isComplete: true, - }, - ]; + const events: AgentConversationEvent[] = completeRetry( + latestConversationTimestamp, + ); if (!event.success && event.finalError) { events.push({ diff --git a/products/desktop/packages/agent/src/server/agent-server.configure-environment.test.ts b/products/desktop/packages/agent/src/server/agent-server.configure-environment.test.ts index 0ae96970f67f..6b18e64f2236 100644 --- a/products/desktop/packages/agent/src/server/agent-server.configure-environment.test.ts +++ b/products/desktop/packages/agent/src/server/agent-server.configure-environment.test.ts @@ -223,6 +223,7 @@ describe("AgentServer.configureEnvironment", () => { "x-posthog-property-task_user_id": "42", "x-posthog-property-task_title": "Fix the bug", "x-posthog-property-team_id": "1", + "x-posthog-property-$ai_session_id": "task-abc", }); }); @@ -340,6 +341,25 @@ describe("AgentServer.configureEnvironment", () => { ); }); + it("folds the task id into the codex session header only", () => { + const env = buildServer("interactive").configureEnvironment({ + taskId: "task-123", + }); + + expect(env.openaiCustomHeaders?.["x-posthog-property-$ai_session_id"]).toBe( + "task-123", + ); + expect(env.anthropicCustomHeaders ?? "").not.toContain("$ai_session_id"); + }); + + it("omits the codex session header without a task id", () => { + const env = buildServer("interactive").configureEnvironment({}); + + expect( + env.openaiCustomHeaders?.["x-posthog-property-$ai_session_id"], + ).toBeUndefined(); + }); + it("appends the resolved product to a LLM_GATEWAY_URL override base", () => { // The override is treated as a base URL. The product slug is always // appended so the gateway routes to the correct product config — a bare diff --git a/products/desktop/packages/agent/src/server/agent-server.ts b/products/desktop/packages/agent/src/server/agent-server.ts index f53594a274fd..e1f075ebcf37 100644 --- a/products/desktop/packages/agent/src/server/agent-server.ts +++ b/products/desktop/packages/agent/src/server/agent-server.ts @@ -3625,6 +3625,10 @@ Optimize for the fewest shell round trips. - Read multiple files at once. - Never rerun a command solely to reproduce output you already have.`; + const artifactInstructions = ` +## Delivering non-code files (artifacts) +When you create a non-code file the user should be able to download (such as a report, chart, image, archive, or data file), call the \`upload_artifact\` tool with its path before your final reply. In your final reply, link to the download URL returned by the tool—never link to the file's local workspace path. Files left in the workspace don't reach the user. Don't upload source code or repository changes—those belong in a commit or PR.`; + const whyContextInstruction = ` - Add a brief **Why** to the body — one or two sentences capturing the reason the user asked for this change (the motivation, not a restatement of the diff). Keep it short.`; const publicRepoSafetyInstruction = ` - **Public-repo safety.** Treat the target repository as public-readable unless you have verified otherwise. The PR title, description, and commit messages must not contain private operational scale (exact event counts, internal row volumes, customer-usage percentages), customer names / emails / companies, references to internal tickets or incidents, the contents of Slack threads (do not quote or paraphrase what was said), or unreleased roadmap details. Linking to the originating Slack thread is fine and encouraged — Slack links are auth-gated and useful as context — as are channel references like "raised in #team-foo". Describe findings qualitatively ("present on nearly all X events, absent from Y") rather than with quantitative figures pulled from analytics queries — the reasoning that uses those numbers can stay in the thread; the PR copy cannot.`; const prMentionSafetyInstruction = ` - **Never guess a GitHub identity.** Do NOT \`@\`-mention, tag, assign, request review from, or attribute the PR to a person (in the title, description, commit message, or reviewers) using a name or handle taken from Slack or this thread. A Slack display name or handle is NOT a GitHub username. Finding a similar-looking handle in the repo's git history, CODEOWNERS, or existing PRs/issues does NOT confirm it belongs to this person: repository presence proves the handle exists, not that it is the person you mean, so treating it as a match still \`@\`-tags an unrelated account (e.g. Slack "Ross" is not necessarily GitHub \`@ross\`, even if some \`@ross\` has committed to the repo). Only \`@\`-mention a GitHub \`@handle\` the user gave you explicitly in this thread. Otherwise refer to people by plain-text name, or omit the mention entirely.`; @@ -3652,7 +3656,7 @@ Do the requested work, but stop with local changes ready for review. Important: - Do NOT create new commits, push to the branch, or update the pull request unless the user explicitly asks. - Do NOT create a new branch or a new pull request unless the user explicitly asks. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} +${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3673,7 +3677,7 @@ After completing the requested changes: Important: - Do NOT create a new branch or a new pull request unless the user explicitly asks. - Do NOT push fixes for review comments without replying to and resolving each related thread. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} +${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3725,7 +3729,7 @@ ${publishInstructions} Important: - Prefer using MCP tools to answer questions with real data over giving generic advice. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} +${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3743,7 +3747,7 @@ ${publicRepoSafetyInstruction.trimStart()} ${prMentionSafetyInstruction.trimStart()} - End the PR description with a horizontal rule followed by this footer line: ${prFooter} - Always create the PR as a draft. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} +${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3771,7 +3775,7 @@ ${prFooter} Important: - Always create the PR as a draft. Do not ask for confirmation. -${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} +${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}${artifactInstructions} `; } @@ -3971,9 +3975,12 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} openaiCustomHeaders = buildGatewayPropertiesHeaderRecord(properties); } else { customHeaders = buildGatewayPropertyHeaders(gatewayProperties); + // No $ai_session_id on the Go-gateway path above: it strips $-prefixed + // blob keys, so the session id would be silently dropped there. openaiCustomHeaders = buildGatewayPropertyHeaderRecord({ ...gatewayProperties, team_id: projectId, + $ai_session_id: taskId, }); } diff --git a/products/desktop/packages/agent/src/session-log-writer.test.ts b/products/desktop/packages/agent/src/session-log-writer.test.ts index 2a492b58ac99..f6931400257c 100644 --- a/products/desktop/packages/agent/src/session-log-writer.test.ts +++ b/products/desktop/packages/agent/src/session-log-writer.test.ts @@ -47,6 +47,45 @@ describe("SessionLogWriter", () => { expect(entries).toHaveLength(2); }); + it("redacts MCP authorization headers before persistence", async () => { + const sessionId = "s1"; + logWriter.register(sessionId, { taskId: "t1", runId: sessionId }); + + logWriter.appendRawLine( + sessionId, + JSON.stringify({ + jsonrpc: "2.0", + method: "session/new", + params: { + mcpServers: [ + { + name: "posthog", + headers: [ + { name: "Authorization", value: "Bearer protocol-secret" }, + { name: "x-posthog-project-id", value: "123" }, + ], + }, + ], + }, + }), + ); + await logWriter.flush(sessionId); + + const entries: StoredNotification[] = mockAppendLog.mock.calls[0][2]; + expect(JSON.stringify(entries)).not.toContain("protocol-secret"); + expect(entries[0].notification.params).toEqual({ + mcpServers: [ + { + name: "posthog", + headers: [ + { name: "Authorization", value: "[REDACTED]" }, + { name: "x-posthog-project-id", value: "123" }, + ], + }, + ], + }); + }); + it("ignores unregistered sessions", async () => { logWriter.appendRawLine("unknown", JSON.stringify({ method: "test" })); await logWriter.flush("unknown"); diff --git a/products/desktop/packages/agent/src/session-log-writer.ts b/products/desktop/packages/agent/src/session-log-writer.ts index a11566902cb0..56bf58440180 100644 --- a/products/desktop/packages/agent/src/session-log-writer.ts +++ b/products/desktop/packages/agent/src/session-log-writer.ts @@ -73,6 +73,31 @@ interface SessionState { pendingRawInputSnapshots: Map; } +function redactAuthorizationHeaders(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(redactAuthorizationHeaders); + } + if (value === null || typeof value !== "object") { + return value; + } + + const record = value as Record; + if ( + typeof record.name === "string" && + record.name.toLowerCase() === "authorization" && + "value" in record + ) { + return { ...record, value: "[REDACTED]" }; + } + + return Object.fromEntries( + Object.entries(record).map(([key, nestedValue]) => [ + key, + redactAuthorizationHeaders(nestedValue), + ]), + ); +} + export class SessionLogWriter { /** * When consecutive in-progress tool updates for one call span more than this @@ -212,7 +237,9 @@ export class SessionLogWriter { const entry: StoredNotification = { type: "notification", timestamp, - notification: message, + notification: redactAuthorizationHeaders( + message, + ) as StoredNotification["notification"], }; this.emitToSinks(sessionId, entry); diff --git a/products/desktop/packages/agent/tsup.config.ts b/products/desktop/packages/agent/tsup.config.ts index 55bfd03fea85..b8167cd2392e 100644 --- a/products/desktop/packages/agent/tsup.config.ts +++ b/products/desktop/packages/agent/tsup.config.ts @@ -132,7 +132,6 @@ export default defineConfig([ "src/pi/rpc-client.ts", "src/pi/runtime.ts", "src/pi/types.ts", - "src/pi/model-catalog.ts", "src/pi/conversation/translatePiConversation.ts", "src/resume.ts", "src/types.ts", diff --git a/products/desktop/packages/api-client/src/mcp-gateway.ts b/products/desktop/packages/api-client/src/mcp-gateway.ts new file mode 100644 index 000000000000..fa6037b27ddb --- /dev/null +++ b/products/desktop/packages/api-client/src/mcp-gateway.ts @@ -0,0 +1,208 @@ +// Types for the team MCP gateway API (`/api/projects/{id}/mcp_gateway/*`). +// Hand-written mirrors of the Django serializers in +// products/mcp_store/backend/presentation/gateway_views.py — these endpoints +// ship behind the `mcp-gateway` flag and are not in the generated OpenAPI +// client yet. +import type { Schemas } from "./generated"; +import type { McpApprovalState, McpAuthType, McpCategory } from "./types"; + +export type McpGatewayUser = Schemas.UserBasic; + +export type McpGatewayScopeType = "team" | "member" | "agent"; +export type McpServiceAccountStatus = "active" | "paused"; +export type McpAuditDecision = "auto" | "approved" | "pending" | "blocked"; +export type McpAuditQuickFilter = "all" | "agents" | "approvals" | "blocked"; +export type McpPolicyDecidedBy = + | "rule" + | "scope" + | "team" + | "preset" + | "legacy" + | "default"; + +/** One member's connection to a gateway server. */ +export interface McpGatewayConnection { + installation_id: string; + user: McpGatewayUser; + last_used_at: string | null; + pending_oauth: boolean; + needs_reauth: boolean; +} + +/** The requesting user's own connection to a gateway server. */ +export interface McpGatewayYourConnection { + installation_id: string; + /** Per-connection switch — false when self-disabled. */ + is_enabled: boolean; + pending_oauth: boolean; + needs_reauth: boolean; + last_used_at: string | null; +} + +/** One agent's access to a gateway server. */ +export interface McpGatewayAgentAccess { + service_account_id: string; + name: string; + /** Agent identity handle, e.g. posthog-support. */ + handle: string; + status: McpServiceAccountStatus; + last_active_at: string | null; + granted_by: McpGatewayUser | null; +} + +/** A server registered in the team's gateway, with connection summary. */ +export interface McpGatewayServer { + id: string; + name: string; + url: string; + description: string; + category: McpCategory; + is_team_enabled: boolean; + icon_key: string; + docs_url: string; + template_id: string | null; + /** + * Fixed authentication type for catalog templates. Null for custom + * servers, where each member chooses when connecting. + */ + template_auth_type: McpAuthType | null; + tool_count: number; + /** Members with a connection to this server. Admin-only; empty for members. */ + connections: McpGatewayConnection[]; + your_connection: McpGatewayYourConnection | null; + agents: McpGatewayAgentAccess[]; + /** Ids of members whose access an admin has turned off. */ + revoked_user_ids: number[]; + is_revoked_for_you: boolean; + created_by: McpGatewayUser | null; + created_at: string; + updated_at: string; +} + +export interface McpGatewayServerUpdate { + name?: string; + description?: string; + category?: McpCategory; + /** Master switch — off means members and agents can neither see nor call the server. */ + is_team_enabled?: boolean; +} + +/** Which policy scope a tools query or policy upsert targets. */ +export interface McpGatewayPolicyScope { + scope_type?: McpGatewayScopeType; + /** Member scope target. Defaults to the requesting user. */ + scope_user_id?: number; + /** Agent scope target. Required when scope_type is "agent". */ + scope_service_account_id?: string; +} + +export interface McpToolPolicyEntry { + tool_name: string; + policy_state: McpApprovalState; +} + +/** One tool with its effective policy for the requested scope. */ +export interface McpResolvedToolPolicy { + tool_name: string; + description: string; + input_schema: unknown; + policy_state: McpApprovalState; + /** What the team-level chain yields, ignoring the scope. Null when the team imposes nothing. */ + team_state: McpApprovalState | null; + /** True when a rule or Blocked team ceiling leaves no editable state. */ + locked: boolean; + decided_by: McpPolicyDecidedBy; + /** Matching org rule name, when decided_by is "rule". */ + rule_name: string; + rule_description: string; +} + +export interface McpServiceAccount { + id: string; + name: string; + description: string; + /** Stable identity handle the agent authenticates as, e.g. posthog-support. */ + handle: string; + status: McpServiceAccountStatus; + /** Masked bearer token; the full token is only shown once. */ + token_mask: string; + server_ids: string[]; + last_active_at: string | null; + created_at: string; + updated_at: string; +} + +export interface McpServiceAccountWithToken extends McpServiceAccount { + /** The full bearer token. Returned exactly once — on creation. */ + token: string; +} + +export interface McpAuditActorServiceAccount { + id: string; + name: string; + handle: string; +} + +export interface McpAuditEvent { + id: string; + created_at: string; + server_name: string; + tool_name: string; + decision: McpAuditDecision; + actor_user: McpGatewayUser | null; + actor_service_account: McpAuditActorServiceAccount | null; + /** Denormalized actor label (email or handle) that survives deletion. */ + actor_label: string; +} + +export interface McpAuditCounts { + all: number; + agents: number; + approvals: number; + blocked: number; +} + +export interface McpAuditPage { + count: number; + results: McpAuditEvent[]; +} + +export interface TeamMcpGatewayConfig { + allow_custom_servers: boolean; + /** Whether members may share MCP connections with agents and manage agent tool policies. */ + allow_member_agent_access: boolean; + /** + * Whether catalog servers the team never touched (no gateway row) are + * enabled. Covers templates published after the admin last curated. + */ + default_servers_enabled: boolean; + /** Whether the requesting user can administer the gateway. */ + is_admin: boolean; +} + +export interface TeamMcpGatewayConfigUpdate { + allow_custom_servers?: boolean; + allow_member_agent_access?: boolean; + default_servers_enabled?: boolean; +} + +/** One team member's gateway posture (admin overview). */ +export interface McpGatewayMemberSummary { + user: McpGatewayUser; + is_org_admin: boolean; + /** Gateway servers the member has a personal connection to. */ + connected_server_ids: string[]; + /** Gateway servers an admin turned off for this member. */ + revoked_server_ids: string[]; +} + +/** + * Gateway options accepted by install_custom / install_template. Credentials + * are always personal to the installer; agents reach them through grants. + */ +export interface McpGatewayInstallSharingOptions { + /** Whether the server starts enabled for the whole team. */ + team_enabled?: boolean; + /** Service accounts to grant the server to at install time, when team settings allow it. */ + agent_ids?: string[]; +} diff --git a/products/desktop/packages/api-client/src/posthog-client.ts b/products/desktop/packages/api-client/src/posthog-client.ts index 05fb47285aae..5007ffc495a3 100644 --- a/products/desktop/packages/api-client/src/posthog-client.ts +++ b/products/desktop/packages/api-client/src/posthog-client.ts @@ -120,11 +120,31 @@ import { requestErrorStatus, } from "./fetcher"; import { createApiClient, type Schemas } from "./generated"; +import type { + McpAuditCounts, + McpAuditEvent, + McpAuditPage, + McpAuditQuickFilter, + McpGatewayInstallSharingOptions, + McpGatewayMemberSummary, + McpGatewayPolicyScope, + McpGatewayServer, + McpGatewayServerUpdate, + McpResolvedToolPolicy, + McpServiceAccount, + McpServiceAccountStatus, + McpServiceAccountWithToken, + McpToolPolicyEntry, + TeamMcpGatewayConfig, + TeamMcpGatewayConfigUpdate, +} from "./mcp-gateway"; import type { SpendAnalysisResponse } from "./spend-analysis"; import { normalizeTaskResponse, normalizeTaskRunResponse, } from "./task-normalization"; + +export type * from "./mcp-gateway"; export interface ApiClientLogger { warn(...args: unknown[]): void; } @@ -4604,17 +4624,19 @@ export class PostHogAPIClient { return data.results ?? []; } - async installCustomMcpServer(options: { - name: string; - url: string; - auth_type: McpAuthType; - api_key?: string; - description?: string; - client_id?: string; - client_secret?: string; - install_source?: "posthog" | "posthog-code"; - posthog_code_callback_url?: string; - }): Promise { + async installCustomMcpServer( + options: { + name: string; + url: string; + auth_type: McpAuthType; + api_key?: string; + description?: string; + client_id?: string; + client_secret?: string; + install_source?: "posthog" | "posthog-code"; + posthog_code_callback_url?: string; + } & McpGatewayInstallSharingOptions, + ): Promise { const teamId = await this.getTeamId(); const apiUrl = new URL( `${this.api.baseUrl}/api/environments/${teamId}/mcp_server_installations/install_custom/`, @@ -4689,12 +4711,14 @@ export class PostHogAPIClient { } } - async installMcpTemplate(options: { - template_id: string; - api_key?: string; - install_source?: "posthog" | "posthog-code"; - posthog_code_callback_url?: string; - }): Promise { + async installMcpTemplate( + options: { + template_id: string; + api_key?: string; + install_source?: "posthog" | "posthog-code"; + posthog_code_callback_url?: string; + } & McpGatewayInstallSharingOptions, + ): Promise { const teamId = await this.getTeamId(); const path = `/api/environments/${teamId}/mcp_server_installations/install_template/`; const response = await this.api.fetcher.fetch({ @@ -4830,6 +4854,303 @@ export class PostHogAPIClient { return data.results ?? []; } + // ---- MCP gateway (team control plane, behind the `mcp-gateway` flag) ---- + + /** + * JSON request against the team-scoped MCP gateway API. `path` is relative + * to `/api/projects/{teamId}/` and must keep its trailing slash. + */ + private async mcpGatewayFetch(args: { + method: "get" | "post" | "patch" | "delete"; + path: string; + search?: Record; + body?: unknown; + errorLabel: string; + }): Promise { + const teamId = await this.getTeamId(); + const path = `/api/projects/${teamId}/${args.path}`; + const url = new URL(`${this.api.baseUrl}${path}`); + for (const [key, value] of Object.entries(args.search ?? {})) { + if (value !== undefined) url.searchParams.set(key, String(value)); + } + const response = await this.api.fetcher.fetch({ + method: args.method, + url, + path, + ...(args.body !== undefined + ? { overrides: { body: JSON.stringify(args.body) } } + : {}), + }); + if (!response.ok && response.status !== 204) { + const errorData = await response.json().catch(() => ({})); + throw new Error( + (errorData as { detail?: string }).detail ?? + `${args.errorLabel}: ${response.statusText}`, + ); + } + if (response.status === 204) return undefined as T; + return (await response.json().catch(() => undefined)) as T; + } + + async getMcpGatewayConfig(): Promise { + return this.mcpGatewayFetch({ + method: "get", + path: "mcp_gateway/config/", + errorLabel: "Failed to fetch gateway settings", + }); + } + + async updateMcpGatewaySettings( + update: TeamMcpGatewayConfigUpdate, + ): Promise { + return this.mcpGatewayFetch({ + method: "post", + path: "mcp_gateway/config/update_settings/", + body: update, + errorLabel: "Failed to update gateway settings", + }); + } + + /** + * Admin: set the team posture for untouched catalog servers and bulk-apply + * the same state to every existing gateway row. + */ + async setAllMcpGatewayServersEnabled( + enabled: boolean, + ): Promise { + return this.mcpGatewayFetch({ + method: "post", + path: "mcp_gateway/config/set_all_servers_enabled/", + body: { enabled }, + errorLabel: "Failed to update servers", + }); + } + + async getMcpGatewayServers(): Promise { + const data = await this.mcpGatewayFetch<{ results?: McpGatewayServer[] }>({ + method: "get", + path: "mcp_gateway/servers/", + search: { limit: 500 }, + errorLabel: "Failed to fetch gateway servers", + }); + return data.results ?? []; + } + + async getMcpGatewayServer(serverId: string): Promise { + return this.mcpGatewayFetch({ + method: "get", + path: `mcp_gateway/servers/${serverId}/`, + errorLabel: "Failed to fetch gateway server", + }); + } + + async updateMcpGatewayServer( + serverId: string, + updates: McpGatewayServerUpdate, + ): Promise { + return this.mcpGatewayFetch({ + method: "patch", + path: `mcp_gateway/servers/${serverId}/`, + body: updates, + errorLabel: "Failed to update gateway server", + }); + } + + /** + * Admin: enable or disable a catalog template the team never touched, + * materializing a gateway row for it (or updating the existing one). + */ + async setMcpGatewayTemplateEnabled(options: { + templateId: string; + enabled: boolean; + }): Promise { + return this.mcpGatewayFetch({ + method: "post", + path: "mcp_gateway/servers/set_template_enabled/", + body: { template_id: options.templateId, enabled: options.enabled }, + errorLabel: "Failed to update catalog server", + }); + } + + /** + * Disconnect every member and delete the row. The registry is sparse, so a + * deleted catalog server simply follows the team default again. + */ + async deleteMcpGatewayServer(serverId: string): Promise { + await this.mcpGatewayFetch({ + method: "delete", + path: `mcp_gateway/servers/${serverId}/`, + errorLabel: "Failed to remove gateway server", + }); + } + + /** Tool catalog with the effective policy resolved for one scope. */ + async getMcpGatewayToolPolicies( + serverId: string, + scope: McpGatewayPolicyScope = {}, + ): Promise { + const data = await this.mcpGatewayFetch<{ + results?: McpResolvedToolPolicy[]; + }>({ + method: "get", + path: `mcp_gateway/servers/${serverId}/tools/`, + search: { + scope_type: scope.scope_type, + scope_user_id: scope.scope_user_id, + scope_service_account_id: scope.scope_service_account_id, + }, + errorLabel: "Failed to fetch tool policies", + }); + return data.results ?? []; + } + + /** Upsert per-tool states for a scope; returns the re-resolved catalog. */ + async upsertMcpGatewayToolPolicies( + serverId: string, + options: McpGatewayPolicyScope & { policies: McpToolPolicyEntry[] }, + ): Promise { + const data = await this.mcpGatewayFetch<{ + results?: McpResolvedToolPolicy[]; + }>({ + method: "post", + path: `mcp_gateway/servers/${serverId}/policies/`, + body: options, + errorLabel: "Failed to update tool policies", + }); + return data.results ?? []; + } + + async getMcpServiceAccounts(): Promise { + const data = await this.mcpGatewayFetch<{ results?: McpServiceAccount[] }>({ + method: "get", + path: "mcp_gateway/service_accounts/", + search: { limit: 500 }, + errorLabel: "Failed to fetch service accounts", + }); + return data.results ?? []; + } + + async getMcpServiceAccount(accountId: string): Promise { + return this.mcpGatewayFetch({ + method: "get", + path: `mcp_gateway/service_accounts/${accountId}/`, + errorLabel: "Failed to fetch service account", + }); + } + + /** Returns the full bearer token exactly once. */ + async createMcpServiceAccount(options: { + name: string; + description?: string; + }): Promise { + return this.mcpGatewayFetch({ + method: "post", + path: "mcp_gateway/service_accounts/", + body: options, + errorLabel: "Failed to create service account", + }); + } + + async updateMcpServiceAccount( + accountId: string, + updates: { + name?: string; + description?: string; + status?: McpServiceAccountStatus; + }, + ): Promise { + return this.mcpGatewayFetch({ + method: "patch", + path: `mcp_gateway/service_accounts/${accountId}/`, + body: updates, + errorLabel: "Failed to update service account", + }); + } + + async deleteMcpServiceAccount(accountId: string): Promise { + await this.mcpGatewayFetch({ + method: "delete", + path: `mcp_gateway/service_accounts/${accountId}/`, + errorLabel: "Failed to delete service account", + }); + } + + /** Grant or revoke one agent's access to one gateway server. */ + async setMcpServiceAccountAccess( + accountId: string, + options: { + gateway_server_id: string; + enabled: boolean; + /** Agent-scope tool policies to set alongside the grant. */ + policies?: McpToolPolicyEntry[]; + }, + ): Promise { + return this.mcpGatewayFetch({ + method: "post", + path: `mcp_gateway/service_accounts/${accountId}/access/`, + body: options, + errorLabel: "Failed to update agent access", + }); + } + + async getMcpGatewayMembers(): Promise { + const data = await this.mcpGatewayFetch<{ + results?: McpGatewayMemberSummary[]; + }>({ + method: "get", + path: "mcp_gateway/members/", + search: { limit: 500 }, + errorLabel: "Failed to fetch gateway members", + }); + return data.results ?? []; + } + + /** Turn one gateway server off (or back on) for one member. */ + async setMcpGatewayMemberAccess( + userId: number, + options: { gateway_server_id: string; enabled: boolean }, + ): Promise { + await this.mcpGatewayFetch({ + method: "post", + path: `mcp_gateway/members/${userId}/set_access/`, + body: options, + errorLabel: "Failed to update member access", + }); + } + + async getMcpGatewayAuditEvents( + options: { + quickFilter?: McpAuditQuickFilter; + actorServiceAccountId?: string; + limit?: number; + offset?: number; + } = {}, + ): Promise { + const data = await this.mcpGatewayFetch<{ + count?: number; + results?: McpAuditEvent[]; + }>({ + method: "get", + path: "mcp_gateway/audit/", + search: { + quick_filter: options.quickFilter, + actor_service_account_id: options.actorServiceAccountId, + limit: options.limit, + offset: options.offset, + }, + errorLabel: "Failed to fetch audit log", + }); + return { count: data.count ?? 0, results: data.results ?? [] }; + } + + async getMcpGatewayAuditCounts(): Promise { + return this.mcpGatewayFetch({ + method: "get", + path: "mcp_gateway/audit/counts/", + errorLabel: "Failed to fetch audit counts", + }); + } + private parseFetcherError(error: unknown): { status: number; body: Record; diff --git a/products/desktop/packages/api-client/src/types.ts b/products/desktop/packages/api-client/src/types.ts index 0c6146081cb4..e63cfd4a1adc 100644 --- a/products/desktop/packages/api-client/src/types.ts +++ b/products/desktop/packages/api-client/src/types.ts @@ -6,8 +6,15 @@ export type McpApprovalState = Schemas.MCPServerInstallationToolApprovalStateEnum; export type McpAuthType = Schemas.MCPAuthTypeEnum; export type McpRecommendedServer = Schemas.MCPServerTemplate; -export type McpServerInstallation = Schemas.MCPServerInstallation; -export type McpInstallationTool = Schemas.MCPServerInstallationTool; +export type McpServerInstallation = Schemas.MCPServerInstallation & { + scope?: "personal" | "shared"; +}; +export type McpInstallationTool = Schemas.MCPServerInstallationTool & { + /** Team-admin ceiling returned by gateway-aware backends. */ + team_state?: McpApprovalState | null; + locked?: boolean; + decided_by?: "rule" | "scope" | "team" | "preset" | "legacy" | "default"; +}; export type McpOAuthRedirectResponse = Schemas.OAuthRedirectResponse; export type McpInstallSource = "posthog" | "posthog-code" | "posthog-mobile"; export type McpInstallResponse = diff --git a/products/desktop/packages/core/src/canvas/channelItems.test.ts b/products/desktop/packages/core/src/canvas/channelItems.test.ts index 59e986179069..63cec64f724d 100644 --- a/products/desktop/packages/core/src/canvas/channelItems.test.ts +++ b/products/desktop/packages/core/src/canvas/channelItems.test.ts @@ -157,6 +157,7 @@ function model(over: Partial = {}): ChannelItemModel { authorName: null, authorUuid: ME.uuid, templateId: null, + task: null, ...over, }; } diff --git a/products/desktop/packages/core/src/canvas/channelItems.ts b/products/desktop/packages/core/src/canvas/channelItems.ts index 4834e6603ac4..9bd9214fce83 100644 --- a/products/desktop/packages/core/src/canvas/channelItems.ts +++ b/products/desktop/packages/core/src/canvas/channelItems.ts @@ -17,6 +17,15 @@ export interface ChannelItemModel { authorName: string | null; authorUuid: string | null; templateId: string | null; + /** + * The source task record for `kind: "task"` rows, `null` for canvases. Rows + * need the whole task, not a projection of it: the status dot is derived from + * session/workspace/viewed state that only the renderer holds, and the hooks + * that supply it (`useChannelTaskData`, `useTaskPrStatus`) take a `Task`. + * Carrying the reference here keeps that a lookup the list already did rather + * than a second pass over every row. + */ + task: Task | null; } export interface ChannelItemOwner { @@ -60,6 +69,7 @@ export function buildChannelItems({ authorName: d.createdBy ?? null, authorUuid: d.createdByUuid ?? null, templateId: d.templateId, + task: null, })); const taskItems: ChannelItemModel[] = feedTasks.flatMap((task) => @@ -78,6 +88,7 @@ export function buildChannelItems({ authorName: null, authorUuid: task.created_by?.uuid ?? null, templateId: null, + task, }, ], ); diff --git a/products/desktop/packages/core/src/cloud-task/cloud-task-engine.ts b/products/desktop/packages/core/src/cloud-task/cloud-task-engine.ts index 05e297f77912..abb90c5789a4 100644 --- a/products/desktop/packages/core/src/cloud-task/cloud-task-engine.ts +++ b/products/desktop/packages/core/src/cloud-task/cloud-task-engine.ts @@ -31,6 +31,12 @@ const SSE_RECONNECT_BASE_DELAY_MS = 500; const SSE_RECONNECT_FLAT_ATTEMPTS = 3; const SSE_RECONNECT_MAX_DELAY_MS = 30_000; const SSE_HEALTHY_CONNECTION_MS = 60_000; +// The backend emits a keepalive at least every ~25-30s (see SSE_KEEPALIVE_INTERVAL_MS in +// packages/agent). A half-open socket (laptop sleep, unplugged NIC, NAT rebind) neither errors +// nor EOFs, so `reader.read()` awaits forever with nothing to trigger reconnect. This timeout +// treats "no bytes at all for a few keepalive intervals" as a disconnect so it flows into the +// existing reconnect/backoff machinery instead of hanging the watcher indefinitely. +const SSE_IDLE_TIMEOUT_MS = 90_000; const EVENT_BATCH_FLUSH_MS = 16; const EVENT_BATCH_MAX_SIZE = 50; const SESSION_LOG_PAGE_LIMIT = 5_000; @@ -1256,12 +1262,75 @@ export class CloudTaskEngine extends TypedEventEmitter { const controller = new AbortController(); watcher.sseAbortController = controller; + let connectedAt = 0; + let streamWasEstablished = false; + let bytesReceived = 0; + let eventsReceived = 0; + let idleTimedOut = false; + let idleTimeoutHandle: ReturnType | null = null; + let idlePhase: "target_resolution" | "connection" | "stream" = + "target_resolution"; + + const clearIdleTimeout = () => { + if (idleTimeoutHandle) { + clearTimeout(idleTimeoutHandle); + idleTimeoutHandle = null; + } + }; + const armIdleTimeout = () => { + clearIdleTimeout(); + idleTimeoutHandle = setTimeout(() => { + idleTimedOut = true; + controller.abort(); + }, SSE_IDLE_TIMEOUT_MS); + }; + const recordIdleTimeout = (details: Record = {}) => { + const idleWatcher = this.watchers.get(key); + this.log.warn("Cloud task stream idle timeout, no bytes received", { + key, + phase: idlePhase, + idleTimeoutMs: SSE_IDLE_TIMEOUT_MS, + bytesReceived, + eventsReceived, + connectionDurationMs: streamWasEstablished + ? Date.now() - connectedAt + : 0, + ...details, + }); + if (idleWatcher) { + this.analytics.track(ANALYTICS_EVENTS.CLOUD_STREAM_IDLE_TIMEOUT, { + task_id: idleWatcher.taskId, + run_id: idleWatcher.runId, + team_id: idleWatcher.teamId, + idle_timeout_ms: SSE_IDLE_TIMEOUT_MS, + bytes_received: bytesReceived, + events_received: eventsReceived, + }); + } + }; + watcher.connStartedAt = 0; watcher.connDataEventsReceived = 0; // Resolve the read target once (proxy URL + token, or Django), reused across reconnects. if (!watcher.streamTargetResolved) { - await this.resolveStreamTarget(watcher); + armIdleTimeout(); + try { + await this.resolveStreamTarget(watcher, controller.signal); + } catch (error) { + if (!idleTimedOut) { + return; + } + recordIdleTimeout(); + await this.handleStreamCompletion(key, { + reconnectOnDisconnect: true, + reconnectError: error, + countReconnectAttempt: true, + }); + return; + } finally { + clearIdleTimeout(); + } const resolvedWatcher = this.watchers.get(key); if ( !resolvedWatcher || @@ -1340,12 +1409,11 @@ export class CloudTaskEngine extends TypedEventEmitter { // Track how long the body stayed open so healthy long-lived connections cut by churn // aren't penalized as failed reconnects (see SSE_HEALTHY_CONNECTION_MS). - let connectedAt = 0; - let streamWasEstablished = false; - let bytesReceived = 0; - let eventsReceived = 0; - + // Re-armed on every read that returns a value (data or keepalive bytes), so it only fires + // when the transport has gone completely silent, not merely between infrequent events. try { + idlePhase = "connection"; + armIdleTimeout(); // The proxy authenticates with the run-scoped Bearer token; the Django leg uses the session. const response = usingProxy ? await this.streamFetch(url.toString(), { @@ -1401,6 +1469,8 @@ export class CloudTaskEngine extends TypedEventEmitter { }); const reader = response.body.getReader(); + idlePhase = "stream"; + armIdleTimeout(); while (true) { const { done, value } = await reader.read(); @@ -1408,6 +1478,8 @@ export class CloudTaskEngine extends TypedEventEmitter { break; } + armIdleTimeout(); + if (!value) { continue; } @@ -1463,10 +1535,20 @@ export class CloudTaskEngine extends TypedEventEmitter { } catch (error) { this.flushLogBatch(key); - if (controller.signal.aborted) { + // An idle-timeout abort must fall through to the reconnect machinery below rather than + // return here like a deliberate cancel (disconnectSse/stopWatching), since nothing else + // will ever notice this connection went silent. + if (controller.signal.aborted && !idleTimedOut) { return; } + if (idleTimedOut) { + recordIdleTimeout({ + leg, + streamUrl: url.toString(), + }); + } + // Proxy-leg 401: the read token expired or its signing key rotated. Re-resolve to mint a // fresh token (or route back to Django) instead of failing. Django-leg 401 stays fatal below. const unauthorizedWatcher = this.watchers.get(key); @@ -1506,6 +1588,7 @@ export class CloudTaskEngine extends TypedEventEmitter { const isBackendError = error instanceof BackendStreamError; const wasHealthyStream = !isBackendError && + !idleTimedOut && streamWasEstablished && Date.now() - connectedAt >= SSE_HEALTHY_CONNECTION_MS; @@ -1548,6 +1631,7 @@ export class CloudTaskEngine extends TypedEventEmitter { countReconnectAttempt: !isBackendError && !wasHealthyStream, }); } finally { + clearIdleTimeout(); const currentWatcher = this.watchers.get(key); if (currentWatcher?.sseAbortController === controller) { currentWatcher.sseAbortController = null; @@ -2198,11 +2282,15 @@ export class CloudTaskEngine extends TypedEventEmitter { } } - private async resolveStreamTarget(watcher: WatcherState): Promise { + private async resolveStreamTarget( + watcher: WatcherState, + signal: AbortSignal, + ): Promise { const url = `${watcher.apiHost}/api/projects/${watcher.teamId}/tasks/${watcher.taskId}/runs/${watcher.runId}/stream_token/`; try { const response = await this.auth.authenticatedFetch(url, { method: "GET", + signal, }); if (!response.ok) { watcher.streamBaseUrl = null; @@ -2245,6 +2333,9 @@ export class CloudTaskEngine extends TypedEventEmitter { durableStream: watcher.durableStreamEnabled, }); } catch (error) { + if (signal.aborted) { + throw error; + } // Transient failure: leave unresolved so the next reconnect retries and falls back to Django. watcher.streamBaseUrl = null; watcher.streamReadToken = null; diff --git a/products/desktop/packages/core/src/cloud-task/cloud-task.test.ts b/products/desktop/packages/core/src/cloud-task/cloud-task.test.ts index 689abbda3709..edd17ec0fa9e 100644 --- a/products/desktop/packages/core/src/cloud-task/cloud-task.test.ts +++ b/products/desktop/packages/core/src/cloud-task/cloud-task.test.ts @@ -94,6 +94,7 @@ async function waitFor( describe("CloudTaskEngine", () => { let service: CloudTaskEngine; + let analyticsMock: { track: ReturnType }; beforeEach(() => { const scopedLog = { @@ -103,7 +104,7 @@ describe("CloudTaskEngine", () => { error: vi.fn(), }; const loggerMock = { ...scopedLog, scope: vi.fn(() => scopedLog) }; - const analyticsMock = { track: vi.fn() }; + analyticsMock = { track: vi.fn() }; service = createCloudTaskEngine({ auth: mockAuthService as never, analytics: analyticsMock as never, @@ -2404,6 +2405,220 @@ describe("CloudTaskEngine", () => { ).toBe(false); }); + it("aborts and reconnects a stream that goes silent with no bytes or keepalives", async () => { + vi.useFakeTimers(); + + const updates: unknown[] = []; + service.on(CloudTaskEvent.Update, (payload) => updates.push(payload)); + + const makeInProgressRun = () => + createJsonResponse({ + id: "run-1", + status: "in_progress", + stage: null, + output: null, + error_message: null, + branch: "main", + updated_at: "2026-01-01T00:00:00Z", + }); + + mockNetFetch + .mockResolvedValueOnce(makeInProgressRun()) + .mockResolvedValueOnce( + createJsonResponse([], 200, { "X-Has-More": "false" }), + ) + .mockImplementation(() => Promise.resolve(makeInProgressRun())); + + // First connection hangs forever: no bytes, no error, no EOF, simulating a half-open + // socket (laptop sleep, NAT rebind). The second connection stays open so recovery is + // observable once the idle watchdog aborts the first. + let streamCall = 0; + const abortedFirstConnection = { value: false }; + mockStreamFetch.mockImplementation( + (_input: unknown, init?: RequestInit) => { + streamCall += 1; + if (streamCall === 1) { + const stream = new ReadableStream({ + start(controller) { + // Never enqueue or close on our own; the read() promise awaits forever until the + // idle watchdog aborts it below, mirroring how a real fetch's reader rejects once + // its AbortSignal fires. + init?.signal?.addEventListener("abort", () => { + abortedFirstConnection.value = true; + controller.error(new DOMException("Aborted", "AbortError")); + }); + }, + }); + return Promise.resolve( + new Response(stream, { + status: 200, + headers: { "Content-Type": "text/event-stream" }, + }), + ); + } + const stream = new ReadableStream({ + start() {}, + }); + return Promise.resolve( + new Response(stream, { + status: 200, + headers: { "Content-Type": "text/event-stream" }, + }), + ); + }, + ); + + service.watch({ + taskId: "task-1", + runId: "run-1", + apiHost: "https://app.example.com", + teamId: 2, + }); + + await waitFor(() => mockStreamFetch.mock.calls.length === 1); + + // Nothing throws or EOFs; without the idle watchdog this would hang forever. + await vi.advanceTimersByTimeAsync(60_000); + expect(abortedFirstConnection.value).toBe(false); + + await vi.advanceTimersByTimeAsync(40_000); + await waitFor(() => abortedFirstConnection.value, 20_000); + await waitFor(() => mockStreamFetch.mock.calls.length >= 2, 20_000); + + expect( + analyticsMock.track.mock.calls.some( + ([eventName]) => eventName === "Cloud stream idle timeout", + ), + ).toBe(true); + + const watcher = ( + service as unknown as { + watchers: Map; + } + ).watchers.get("task-1:run-1"); + expect(watcher?.failed).toBe(false); + // Silence is a broken transport, not a healthy long-lived connection. It must consume the + // reconnect budget so a persistently silent endpoint eventually reaches the circuit breaker. + expect(watcher?.reconnectAttempts).toBe(1); + expect( + updates.some( + (u) => + typeof u === "object" && + u !== null && + (u as { kind?: string }).kind === "error", + ), + ).toBe(false); + }); + + it("times out while resolving the stream target and reconnects", async () => { + vi.useFakeTimers(); + + mockNetFetch + .mockResolvedValueOnce( + createJsonResponse({ id: "run-1", status: "in_progress" }), + ) + .mockResolvedValueOnce( + createJsonResponse([], 200, { "X-Has-More": "false" }), + ) + .mockResolvedValue( + createJsonResponse({ id: "run-1", status: "in_progress" }), + ); + + let tokenCall = 0; + const abortedResolution = { value: false }; + mockStreamTokenFetch.mockImplementation( + (_input: unknown, init?: RequestInit) => { + tokenCall += 1; + if (tokenCall === 1) { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => { + abortedResolution.value = true; + reject(new DOMException("Aborted", "AbortError")); + }); + }); + } + return Promise.resolve( + createJsonResponse({ token: "test-token", stream_base_url: null }), + ); + }, + ); + mockStreamFetch.mockImplementation( + () => + new Promise(() => { + // The connection-phase watchdog owns this second pending request. + }), + ); + + service.watch({ + taskId: "task-1", + runId: "run-1", + apiHost: "https://app.example.com", + teamId: 2, + }); + + await waitFor(() => mockStreamTokenFetch.mock.calls.length === 1); + await vi.advanceTimersByTimeAsync(100_000); + await waitFor(() => abortedResolution.value, 20_000); + await waitFor(() => mockStreamTokenFetch.mock.calls.length >= 2, 20_000); + + expect(mockStreamTokenFetch.mock.calls[0]?.[1]?.signal).toBeDefined(); + expect( + analyticsMock.track.mock.calls.some( + ([eventName]) => eventName === "Cloud stream idle timeout", + ), + ).toBe(true); + }); + + it("times out while waiting for stream response headers and reconnects", async () => { + vi.useFakeTimers(); + + mockNetFetch + .mockResolvedValueOnce( + createJsonResponse({ id: "run-1", status: "in_progress" }), + ) + .mockResolvedValueOnce( + createJsonResponse([], 200, { "X-Has-More": "false" }), + ) + .mockResolvedValue( + createJsonResponse({ id: "run-1", status: "in_progress" }), + ); + + let streamCall = 0; + const abortedConnection = { value: false }; + mockStreamFetch.mockImplementation( + (_input: unknown, init?: RequestInit) => { + streamCall += 1; + if (streamCall === 1) { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => { + abortedConnection.value = true; + reject(new DOMException("Aborted", "AbortError")); + }); + }); + } + return Promise.resolve(createOpenSseResponse("")); + }, + ); + + service.watch({ + taskId: "task-1", + runId: "run-1", + apiHost: "https://app.example.com", + teamId: 2, + }); + + await waitFor(() => mockStreamFetch.mock.calls.length === 1); + await vi.advanceTimersByTimeAsync(100_000); + await waitFor(() => abortedConnection.value, 20_000); + await waitFor(() => mockStreamFetch.mock.calls.length >= 2, 20_000); + + expect( + analyticsMock.track.mock.calls.some( + ([eventName]) => eventName === "Cloud stream idle timeout", + ), + ).toBe(true); + }); + it("stops a cloud run through the run cancel endpoint", async () => { mockNetFetch.mockResolvedValueOnce( createJsonResponse({ id: "run-1", status: "in_progress" }, 202), diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayAddServer.test.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayAddServer.test.ts new file mode 100644 index 000000000000..b59815ff0bc9 --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayAddServer.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from "vitest"; +import { + buildGatewayInstallRequest, + canSubmitGatewayServer, + GATEWAY_ADD_SERVER_DEFAULTS, + type GatewayAddServerValues, +} from "./gatewayAddServer"; + +function values( + overrides: Partial = {}, +): GatewayAddServerValues { + return { + ...GATEWAY_ADD_SERVER_DEFAULTS, + name: "Internal Wiki", + url: "https://mcp.example.com/sse", + ...overrides, + }; +} + +describe("canSubmitGatewayServer", () => { + it.each([ + ["valid name and url", values(), true], + ["missing name", values({ name: " " }), false], + ["invalid url", values({ url: "not-a-url" }), false], + ])("%s", (_label, input, expected) => { + expect(canSubmitGatewayServer(input)).toBe(expected); + }); +}); + +describe("buildGatewayInstallRequest", () => { + it("builds an oauth install with admin team options", () => { + const request = buildGatewayInstallRequest( + values({ description: " Wiki tools ", agentIds: ["svc-1"] }), + { isAdmin: true, canManageAgentAccess: true }, + ); + expect(request).toEqual({ + name: "Internal Wiki", + url: "https://mcp.example.com/sse", + description: "Wiki tools", + auth_type: "oauth", + team_enabled: true, + agent_ids: ["svc-1"], + }); + }); + + it("includes the key on api-key installs", () => { + const request = buildGatewayInstallRequest( + values({ authType: "api_key", apiKey: "sk-123" }), + { isAdmin: true, canManageAgentAccess: true }, + ); + expect(request.auth_type).toBe("api_key"); + expect(request.api_key).toBe("sk-123"); + }); + + it("includes oauth client credentials only when provided", () => { + const bare = buildGatewayInstallRequest(values(), { + isAdmin: true, + canManageAgentAccess: true, + }); + expect(bare.client_id).toBeUndefined(); + const withCreds = buildGatewayInstallRequest( + values({ clientId: " id ", clientSecret: "secret" }), + { isAdmin: true, canManageAgentAccess: true }, + ); + expect(withCreds.client_id).toBe("id"); + expect(withCreds.client_secret).toBe("secret"); + }); + + it("lets permitted members share with agents without team enablement", () => { + const request = buildGatewayInstallRequest( + values({ agentIds: ["svc-1"] }), + { isAdmin: false, canManageAgentAccess: true }, + ); + expect(request.team_enabled).toBeUndefined(); + expect(request.agent_ids).toEqual(["svc-1"]); + }); + + it("omits agent grants when team settings make them admin-only", () => { + const request = buildGatewayInstallRequest( + values({ agentIds: ["svc-1"] }), + { isAdmin: false, canManageAgentAccess: false }, + ); + expect(request.agent_ids).toBeUndefined(); + }); +}); diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayAddServer.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayAddServer.ts new file mode 100644 index 000000000000..23870376d7f9 --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayAddServer.ts @@ -0,0 +1,77 @@ +import type { + McpAuthType, + McpGatewayInstallSharingOptions, +} from "@posthog/api-client/posthog-client"; +import { isValidMcpUrl } from "../mcp-servers/customServerForm"; + +export interface GatewayAddServerValues { + name: string; + url: string; + description: string; + authType: McpAuthType; + apiKey: string; + clientId: string; + clientSecret: string; + /** Team sharing options are admin-only; agentIds follows the team setting. */ + teamEnabled: boolean; + agentIds: string[]; +} + +export const GATEWAY_ADD_SERVER_DEFAULTS: GatewayAddServerValues = { + name: "", + url: "", + description: "", + authType: "oauth", + apiKey: "", + clientId: "", + clientSecret: "", + teamEnabled: true, + agentIds: [], +}; + +export function canSubmitGatewayServer( + values: Pick, +): boolean { + return values.name.trim() !== "" && isValidMcpUrl(values.url); +} + +export interface GatewayInstallRequest extends McpGatewayInstallSharingOptions { + name: string; + url: string; + description: string; + auth_type: McpAuthType; + api_key?: string; + client_id?: string; + client_secret?: string; +} + +/** + * install_custom payload for registering a server with the gateway. The + * credential is always personal to the installer. Team-wide options are + * attached only for admins; agent grants are attached whenever the team + * allows this member to manage agent access. + */ +export function buildGatewayInstallRequest( + values: GatewayAddServerValues, + options: { isAdmin: boolean; canManageAgentAccess: boolean }, +): GatewayInstallRequest { + return { + name: values.name.trim(), + url: values.url.trim(), + description: values.description.trim(), + auth_type: values.authType, + ...(values.authType === "api_key" && values.apiKey + ? { api_key: values.apiKey } + : {}), + ...(values.authType === "oauth" && values.clientId.trim() + ? { client_id: values.clientId.trim() } + : {}), + ...(values.authType === "oauth" && values.clientSecret.trim() + ? { client_secret: values.clientSecret.trim() } + : {}), + ...(options.isAdmin ? { team_enabled: values.teamEnabled } : {}), + ...(options.canManageAgentAccess && values.agentIds.length + ? { agent_ids: values.agentIds } + : {}), + }; +} diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayConnect.test.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayConnect.test.ts new file mode 100644 index 000000000000..67a27f7e1f13 --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayConnect.test.ts @@ -0,0 +1,215 @@ +import { describe, expect, it, vi } from "vitest"; +import type { + InstallFlowClient, + IOAuthCallback, +} from "../mcp-servers/installFlow"; +import { + canSubmitGatewayConnect, + connectGatewayServer, + GATEWAY_CONNECT_DEFAULTS, + type GatewayConnectCredentials, + gatewayConnectAuthType, + gatewayConnectNeedsCredentials, + templateConnectNeedsCredentials, +} from "./gatewayConnect"; + +function credentials( + overrides: Partial = {}, +): GatewayConnectCredentials { + return { ...GATEWAY_CONNECT_DEFAULTS, ...overrides }; +} + +function fakes() { + const client: InstallFlowClient = { + installMcpTemplate: vi.fn().mockResolvedValue({ id: "inst-1" }), + installCustomMcpServer: vi.fn().mockResolvedValue({ id: "inst-2" }), + authorizeMcpInstallation: vi + .fn() + .mockResolvedValue({ redirect_url: "https://auth" }), + }; + const oauth: IOAuthCallback = { + getCallbackUrl: vi + .fn() + .mockResolvedValue({ callbackUrl: "posthog://callback" }), + openAndWaitForCallback: vi.fn().mockResolvedValue({ success: true }), + }; + return { client, oauth }; +} + +describe("gatewayConnectAuthType", () => { + it.each([ + [ + "oauth template", + { template_id: "t1", template_auth_type: "oauth" }, + "oauth", + ], + [ + "api-key template", + { template_id: "t1", template_auth_type: "api_key" }, + "api_key", + ], + [ + "template with no reported type", + { template_id: "t1", template_auth_type: null }, + "oauth", + ], + [ + "custom server — member chooses", + { template_id: null, template_auth_type: null }, + null, + ], + ] as const)("%s", (_label, server, expected) => { + expect(gatewayConnectAuthType(server)).toBe(expected); + }); +}); + +describe("gatewayConnectNeedsCredentials", () => { + it.each([ + [ + "oauth template connects directly", + { template_id: "t1", template_auth_type: "oauth" }, + false, + ], + [ + "api-key template asks for the key", + { template_id: "t1", template_auth_type: "api_key" }, + true, + ], + [ + "custom server asks the member to choose", + { template_id: null, template_auth_type: null }, + true, + ], + ] as const)("%s", (_label, server, expected) => { + expect(gatewayConnectNeedsCredentials(server)).toBe(expected); + }); +}); + +describe("templateConnectNeedsCredentials", () => { + it.each([ + ["oauth template", { auth_type: "oauth" }, false], + ["api-key template", { auth_type: "api_key" }, true], + ["auth type unreported", {}, false], + ] as const)("%s", (_label, template, expected) => { + expect( + templateConnectNeedsCredentials( + template as { auth_type?: "oauth" | "api_key" }, + ), + ).toBe(expected); + }); +}); + +describe("canSubmitGatewayConnect", () => { + it.each([ + ["oauth needs no key", credentials(), true], + [ + "api_key with a key", + credentials({ authType: "api_key", apiKey: "sk-1" }), + true, + ], + [ + "api_key with a blank key", + credentials({ authType: "api_key", apiKey: " " }), + false, + ], + ])("%s", (_label, input, expected) => { + expect(canSubmitGatewayConnect(input)).toBe(expected); + }); +}); + +describe("connectGatewayServer", () => { + const template = { + template_id: "t1", + name: "Linear", + url: "https://mcp.linear.app", + description: "", + }; + const custom = { + template_id: null, + name: "Internal Wiki", + url: "https://mcp.example.com/sse", + description: "Wiki tools", + }; + + it("installs a template with the member's API key", async () => { + const { client, oauth } = fakes(); + await connectGatewayServer( + client, + oauth, + template, + credentials({ authType: "api_key", apiKey: "sk-1" }), + ); + expect(client.installMcpTemplate).toHaveBeenCalledWith({ + template_id: "t1", + api_key: "sk-1", + install_source: "posthog-code", + posthog_code_callback_url: "posthog://callback", + }); + expect(client.installCustomMcpServer).not.toHaveBeenCalled(); + }); + + it("installs an oauth template without a key by default", async () => { + const { client, oauth } = fakes(); + await connectGatewayServer(client, oauth, template); + expect(client.installMcpTemplate).toHaveBeenCalledWith({ + template_id: "t1", + api_key: undefined, + install_source: "posthog-code", + posthog_code_callback_url: "posthog://callback", + }); + }); + + it("connects a custom server with the chosen api_key mechanism", async () => { + const { client, oauth } = fakes(); + const result = await connectGatewayServer( + client, + oauth, + custom, + credentials({ + authType: "api_key", + apiKey: "sk-2", + // A stale value from flipping the auth select must not leak through. + clientId: "leftover", + clientSecret: "leftover", + }), + ); + expect(client.installCustomMcpServer).toHaveBeenCalledWith({ + name: "Internal Wiki", + url: "https://mcp.example.com/sse", + description: "Wiki tools", + auth_type: "api_key", + api_key: "sk-2", + client_id: undefined, + client_secret: undefined, + install_source: "posthog-code", + posthog_code_callback_url: "posthog://callback", + }); + // API-key installs return no redirect, so no browser round-trip. + expect(oauth.openAndWaitForCallback).not.toHaveBeenCalled(); + expect(result).toEqual({ success: true }); + }); + + it("connects a custom server over oauth with optional client credentials", async () => { + const { client, oauth } = fakes(); + vi.mocked(client.installCustomMcpServer).mockResolvedValue({ + redirect_url: "https://auth.example.com", + }); + await connectGatewayServer( + client, + oauth, + custom, + credentials({ clientId: " id ", clientSecret: "secret" }), + ); + expect(client.installCustomMcpServer).toHaveBeenCalledWith( + expect.objectContaining({ + auth_type: "oauth", + api_key: undefined, + client_id: "id", + client_secret: "secret", + }), + ); + expect(oauth.openAndWaitForCallback).toHaveBeenCalledWith({ + redirectUrl: "https://auth.example.com", + }); + }); +}); diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayConnect.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayConnect.ts new file mode 100644 index 000000000000..798b4d87fae7 --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayConnect.ts @@ -0,0 +1,113 @@ +import type { + McpAuthType, + McpGatewayServer, +} from "@posthog/api-client/posthog-client"; +import { + type InstallFlowClient, + type IOAuthCallback, + installCustomWithOAuth, + installTemplateWithOAuth, + type OAuthCallbackResult, +} from "../mcp-servers/installFlow"; + +/** Personal credentials a member supplies when connecting to a gateway server. */ +export interface GatewayConnectCredentials { + authType: McpAuthType; + apiKey: string; + /** Optional OAuth client for providers without dynamic client registration. */ + clientId: string; + clientSecret: string; +} + +export const GATEWAY_CONNECT_DEFAULTS: GatewayConnectCredentials = { + authType: "oauth", + apiKey: "", + clientId: "", + clientSecret: "", +}; + +type GatewayConnectServer = Pick< + McpGatewayServer, + "template_id" | "template_auth_type" +>; + +/** + * The auth mechanism a connection must use: catalog templates fix it, custom + * servers leave it null — every credential is personal, so each member picks + * their own mechanism when they connect. + */ +export function gatewayConnectAuthType( + server: GatewayConnectServer, +): McpAuthType | null { + return server.template_id ? (server.template_auth_type ?? "oauth") : null; +} + +/** + * OAuth connects need no input up front — the browser round-trip collects the + * grant. Everything else (API-key templates, custom servers where the member + * chooses) must collect credentials before installing. + */ +export function gatewayConnectNeedsCredentials( + server: GatewayConnectServer, +): boolean { + return gatewayConnectAuthType(server) !== "oauth"; +} + +/** Same decision for a catalog template with no gateway row yet. */ +export function templateConnectNeedsCredentials(template: { + auth_type?: McpAuthType; +}): boolean { + return (template.auth_type ?? "oauth") !== "oauth"; +} + +export function canSubmitGatewayConnect( + credentials: Pick, +): boolean { + return credentials.authType !== "api_key" || credentials.apiKey.trim() !== ""; +} + +export interface GatewayConnectTarget { + template_id: string | null; + name: string; + url: string; + description: string; +} + +/** + * Connect the caller's own credential to a gateway server, or to a catalog + * template with no row yet — the backend materializes the row. Honors the + * chosen auth mechanism instead of assuming OAuth: API-key connects complete + * inline, OAuth connects round-trip the host browser callback. + */ +export async function connectGatewayServer( + client: InstallFlowClient, + oauth: IOAuthCallback, + target: GatewayConnectTarget, + credentials: GatewayConnectCredentials = GATEWAY_CONNECT_DEFAULTS, +): Promise { + const apiKey = + credentials.authType === "api_key" && credentials.apiKey + ? credentials.apiKey + : undefined; + if (target.template_id) { + return installTemplateWithOAuth(client, oauth, { + template_id: target.template_id, + api_key: apiKey, + }); + } + return installCustomWithOAuth(client, oauth, { + name: target.name, + url: target.url, + description: target.description, + auth_type: credentials.authType, + api_key: apiKey, + client_id: + credentials.authType === "oauth" && credentials.clientId.trim() + ? credentials.clientId.trim() + : undefined, + client_secret: + credentials.authType === "oauth" && credentials.clientSecret.trim() + ? credentials.clientSecret.trim() + : undefined, + }); +} diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayInstallFlow.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayInstallFlow.ts new file mode 100644 index 000000000000..5c1a22d89b33 --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayInstallFlow.ts @@ -0,0 +1,40 @@ +import type { McpServerInstallation } from "@posthog/api-client/types"; +import type { + IOAuthCallback, + OAuthCallbackResult, +} from "../mcp-servers/installFlow"; +import type { GatewayInstallRequest } from "./gatewayAddServer"; + +interface OAuthRedirect { + redirect_url: string; +} + +interface GatewayInstallClient { + installCustomMcpServer( + options: GatewayInstallRequest & { + install_source?: "posthog" | "posthog-code"; + posthog_code_callback_url?: string; + }, + ): Promise; +} + +/** + * Register a custom server with the gateway. OAuth servers round-trip through + * the host browser callback; API-key servers complete immediately. + */ +export async function registerGatewayServerWithOAuth( + client: GatewayInstallClient, + oauth: IOAuthCallback, + request: GatewayInstallRequest, +): Promise { + const { callbackUrl } = await oauth.getCallbackUrl(); + const data = await client.installCustomMcpServer({ + ...request, + install_source: "posthog-code", + posthog_code_callback_url: callbackUrl, + }); + if ("redirect_url" in data && data.redirect_url) { + return oauth.openAndWaitForCallback({ redirectUrl: data.redirect_url }); + } + return { success: true }; +} diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayServers.test.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayServers.test.ts new file mode 100644 index 000000000000..e4be219d718e --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayServers.test.ts @@ -0,0 +1,491 @@ +import type { + McpGatewayServer, + McpGatewayYourConnection, + McpResolvedToolPolicy, +} from "@posthog/api-client/posthog-client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + countGatewayServersByCategory, + countPoliciesByState, + defaultAgentGrantPolicy, + filterCatalogTemplates, + filterGatewayServers, + formatAgo, + formatAuditTime, + getGatewayConnectionStatus, + getGatewayRailStatus, + getGatewayServerRemovalAction, + isAgentPolicyState, + isConnectedForYou, + isPolicyStateAllowedByCeiling, + normalizeGatewayServerUrl, + railConnectedServers, + recommendedCatalogTemplates, + resolvePolicyStateForScope, +} from "./gatewayServers"; + +describe("agent tool policies", () => { + it.each([ + ["approved", true], + ["needs_approval", false], + ["do_not_use", true], + ] as const)("allows %s for agents: %s", (state, expected) => { + expect(isAgentPolicyState(state)).toBe(expected); + }); + + it("treats approval-gated tools as blocked for agents only", () => { + expect(resolvePolicyStateForScope("needs_approval", "agent")).toBe( + "do_not_use", + ); + expect(resolvePolicyStateForScope("needs_approval", "member")).toBe( + "needs_approval", + ); + expect(resolvePolicyStateForScope("needs_approval", "team")).toBe( + "needs_approval", + ); + }); +}); + +describe("isPolicyStateAllowedByCeiling", () => { + it.each([ + ["approved", "needs_approval", false], + ["needs_approval", "needs_approval", true], + ["do_not_use", "needs_approval", true], + ["approved", "do_not_use", false], + ["needs_approval", "do_not_use", false], + ["do_not_use", "do_not_use", true], + ["approved", "approved", true], + ["approved", null, true], + ] as const)("%s under a %s ceiling is %s", (state, ceiling, expected) => { + expect(isPolicyStateAllowedByCeiling(state, ceiling)).toBe(expected); + }); +}); + +function connection( + overrides: Partial = {}, +): McpGatewayYourConnection { + return { + installation_id: "inst-1", + is_enabled: true, + pending_oauth: false, + needs_reauth: false, + last_used_at: null, + ...overrides, + }; +} + +function server(overrides: Partial): McpGatewayServer { + return { + id: "srv-1", + name: "Test", + url: "https://mcp.example.com", + description: "", + category: "dev", + is_team_enabled: true, + icon_key: "", + docs_url: "", + template_id: null, + template_auth_type: null, + tool_count: 0, + connections: [], + your_connection: null, + agents: [], + revoked_user_ids: [], + is_revoked_for_you: false, + created_by: null, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + ...overrides, + }; +} + +describe("railConnectedServers", () => { + const servers = [ + server({ id: "a", name: "Alpha", your_connection: connection() }), + server({ id: "b", name: "Beta" }), + server({ id: "c", name: "Gamma", your_connection: connection() }), + ]; + + it("lists only the servers the caller has connected", () => { + expect(railConnectedServers(servers, "").map((s) => s.id)).toEqual([ + "a", + "c", + ]); + }); + + it("filters by name", () => { + expect(railConnectedServers(servers, "gam").map((s) => s.id)).toEqual([ + "c", + ]); + }); +}); + +describe("filterGatewayServers", () => { + const servers = [ + server({ id: "a", name: "Linear", description: "Ticket tracker" }), + server({ + id: "b", + name: "GitHub", + description: "Code hosting", + category: "data", + }), + server({ id: "c", name: "Notion", url: "https://mcp.notion.so" }), + ]; + + it("matches name, description and url case-insensitively", () => { + expect(filterGatewayServers(servers, "TICKET", null)[0]?.id).toBe("a"); + expect(filterGatewayServers(servers, "notion.so", null)[0]?.id).toBe("c"); + }); + + it("applies the category chip", () => { + expect(filterGatewayServers(servers, "", "data").map((s) => s.id)).toEqual([ + "b", + ]); + }); + + it("combines search and category", () => { + expect(filterGatewayServers(servers, "linear", "data")).toEqual([]); + }); +}); + +describe("filterCatalogTemplates", () => { + const templates = [ + { id: "t1", name: "Linear", description: "Tickets", url: "https://a" }, + { id: "t2", name: "GitHub", url: "https://b", category: "data" }, + ]; + + it("matches name/description/url and tolerates missing fields", () => { + expect(filterCatalogTemplates(templates, "tickets", null)).toEqual([ + templates[0], + ]); + expect(filterCatalogTemplates(templates, "https://b", null)).toEqual([ + templates[1], + ]); + }); + + it("applies the category chip", () => { + expect(filterCatalogTemplates(templates, "", "data")).toEqual([ + templates[1], + ]); + }); +}); + +describe("normalizeGatewayServerUrl", () => { + it.each([ + ["https://mcp.linear.app/sse/", "https://mcp.linear.app/sse"], + ["https://mcp.linear.app/sse", "https://mcp.linear.app/sse"], + [" https://mcp.linear.app// ", "https://mcp.linear.app"], + ])("normalizes %s", (input, expected) => { + expect(normalizeGatewayServerUrl(input)).toBe(expected); + }); +}); + +describe("recommendedCatalogTemplates", () => { + const templates = [ + { id: "t1", url: "https://mcp.linear.app/sse" }, + { id: "t2", url: "https://mcp.notion.so/mcp" }, + { id: "t3", url: "https://mcp.stripe.com" }, + ]; + + it("excludes templates already materialized by template id", () => { + const servers = [server({ template_id: "t1", url: "https://elsewhere" })]; + expect(recommendedCatalogTemplates(servers, templates)).toEqual([ + templates[1], + templates[2], + ]); + }); + + it("excludes templates matched by trailing-slash-insensitive url", () => { + const servers = [ + server({ template_id: null, url: "https://mcp.notion.so/mcp/" }), + ]; + expect(recommendedCatalogTemplates(servers, templates)).toEqual([ + templates[0], + templates[2], + ]); + }); + + it("returns every template when the registry is empty", () => { + expect(recommendedCatalogTemplates([], templates)).toEqual(templates); + }); +}); + +describe("countGatewayServersByCategory", () => { + it("tallies per category", () => { + const counts = countGatewayServersByCategory([ + server({ id: "a", category: "dev" }), + server({ id: "b", category: "dev" }), + server({ id: "c", category: "data" }), + ]); + expect(counts).toEqual({ dev: 2, data: 1 }); + }); +}); + +describe("isConnectedForYou", () => { + it.each([ + ["own connection", server({ your_connection: connection() }), true], + [ + "pending oauth does not count", + server({ your_connection: connection({ pending_oauth: true }) }), + false, + ], + [ + "connection needing reauth does not count", + server({ your_connection: connection({ needs_reauth: true }) }), + false, + ], + ["not connected", server({}), false], + ] as const)("%s", (_label, srv, expected) => { + expect(isConnectedForYou(srv)).toBe(expected); + }); +}); + +describe("getGatewayConnectionStatus", () => { + it.each([ + ["connected", connection(), "connected"], + ["pending OAuth", connection({ pending_oauth: true }), "pending_oauth"], + [ + "needs reauthorization", + connection({ needs_reauth: true }), + "needs_reauth", + ], + [ + "reauthorization takes precedence when both flags are set", + connection({ pending_oauth: true, needs_reauth: true }), + "needs_reauth", + ], + ] as const)("returns the status for %s", (_label, value, expected) => { + expect(getGatewayConnectionStatus(value)).toBe(expected); + }); +}); + +describe("getGatewayRailStatus", () => { + it.each([ + ["no connection", server({}), null], + [ + "a usable connection", + server({ your_connection: connection() }), + "connected", + ], + [ + "a connection pending OAuth", + server({ your_connection: connection({ pending_oauth: true }) }), + "pending_oauth", + ], + [ + "a connection needing reauthorization", + server({ your_connection: connection({ needs_reauth: true }) }), + "needs_reauth", + ], + [ + "a self-disabled connection", + server({ your_connection: connection({ is_enabled: false }) }), + "self_disabled", + ], + [ + "revoked access", + server({ is_revoked_for_you: true, your_connection: connection() }), + "revoked", + ], + [ + "a team-disabled server", + server({ is_team_enabled: false, your_connection: connection() }), + "team_off", + ], + [ + "self-disabled outranking the auth states", + server({ + your_connection: connection({ is_enabled: false, needs_reauth: true }), + }), + "self_disabled", + ], + [ + "revocation outranking self-disable", + server({ + is_revoked_for_you: true, + your_connection: connection({ is_enabled: false }), + }), + "revoked", + ], + [ + "the team master switch outranking everything", + server({ + is_team_enabled: false, + is_revoked_for_you: true, + your_connection: connection({ is_enabled: false, needs_reauth: true }), + }), + "team_off", + ], + ] as const)("returns the status for %s", (_label, srv, expected) => { + expect(getGatewayRailStatus(srv)).toBe(expected); + }); +}); + +describe("getGatewayServerRemovalAction", () => { + const gatewayUser = (id: number) => ({ + id, + uuid: `user-${id}`, + email: `user-${id}@example.com`, + hedgehog_config: null, + }); + + // Members never receive `connections` (it is admin-only), so every + // non-admin case keeps the default empty roster — the real API shape. + it.each([ + [ + "deletes a personally added custom server", + server({ + created_by: gatewayUser(1), + your_connection: connection(), + }), + false, + 1, + "delete_for_you", + ], + [ + "disconnects from a custom server added by someone else", + server({ + created_by: gatewayUser(2), + your_connection: connection(), + }), + false, + 1, + "disconnect", + ], + [ + "disconnects from a custom server with no recorded creator", + server({ + created_by: null, + your_connection: connection(), + }), + false, + 1, + "disconnect", + ], + [ + "disconnects when the current user is unknown", + server({ + created_by: gatewayUser(1), + your_connection: connection(), + }), + false, + null, + "disconnect", + ], + [ + "disconnects from a catalog server", + server({ + template_id: "template-1", + created_by: gatewayUser(1), + your_connection: connection(), + }), + false, + 1, + "disconnect", + ], + [ + "deletes a custom server for everyone when requested by an admin", + server({}), + true, + 1, + "delete_for_everyone", + ], + [ + "does not delete a catalog server for an admin without a connection", + server({ template_id: "template-1" }), + true, + 1, + null, + ], + [ + "returns no action without a personal connection", + server({}), + false, + 1, + null, + ], + ] as const)("%s", (_label, srv, isAdmin, currentUserId, expected) => { + expect(getGatewayServerRemovalAction(srv, isAdmin, currentUserId)).toBe( + expected, + ); + }); +}); + +describe("countPoliciesByState", () => { + it("counts each state, defaulting to zero", () => { + const policy = (state: McpResolvedToolPolicy["policy_state"]) => + ({ + tool_name: "t", + description: "", + input_schema: {}, + policy_state: state, + team_state: null, + locked: false, + decided_by: "default", + rule_name: "", + rule_description: "", + }) satisfies McpResolvedToolPolicy; + expect( + countPoliciesByState([ + policy("approved"), + policy("approved"), + policy("do_not_use"), + ]), + ).toEqual({ approved: 2, needs_approval: 0, do_not_use: 1 }); + }); + + it("counts approval-gated agent tools as blocked", () => { + expect( + countPoliciesByState( + [ + { + tool_name: "send_message", + description: "", + input_schema: {}, + policy_state: "needs_approval", + team_state: null, + locked: false, + decided_by: "scope", + rule_name: "", + rule_description: "", + }, + ], + "agent", + ), + ).toEqual({ approved: 0, needs_approval: 0, do_not_use: 1 }); + }); +}); + +describe("time formatting", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-07-21T12:00:00")); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + it("formatAgo renders short relative times", () => { + expect(formatAgo("2026-07-21T10:00:00")).toBe("2h ago"); + expect(formatAgo("2026-07-21T11:59:50")).toBe("just now"); + expect(formatAgo(null)).toBeNull(); + }); + + it("formatAuditTime buckets by local day", () => { + expect(formatAuditTime("2026-07-21T09:58:00")).toBe("Today 09:58"); + expect(formatAuditTime("2026-07-20T17:22:00")).toBe("Yesterday 17:22"); + expect(formatAuditTime("2026-07-15T09:12:00")).toMatch(/^Jul 15 09:12$/); + }); +}); + +describe("defaultAgentGrantPolicy", () => { + it.each([ + ["delete-row", "do_not_use"], + ["run-migration", "do_not_use"], + ["send", "do_not_use"], + ["list-tables", "approved"], + ["search", "approved"], + ] as const)("%s → %s", (tool, expected) => { + expect(defaultAgentGrantPolicy(tool)).toBe(expected); + }); +}); diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayServers.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayServers.ts new file mode 100644 index 000000000000..cb70f8594904 --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayServers.ts @@ -0,0 +1,292 @@ +import type { + McpApprovalState, + McpAuditDecision, + McpGatewayScopeType, + McpGatewayServer, + McpGatewayYourConnection, + McpResolvedToolPolicy, +} from "@posthog/api-client/posthog-client"; +import { formatRelativeTimeShort, getLocalDayDiff } from "@posthog/shared"; + +/** The rail lists the servers the caller has connected, under the rail search. */ +export function railConnectedServers( + servers: McpGatewayServer[], + search: string, +): McpGatewayServer[] { + const query = search.trim().toLowerCase(); + return servers.filter( + (server) => + server.your_connection !== null && + (!query || server.name.toLowerCase().includes(query)), + ); +} + +interface GatewayServerLike { + name: string; + description?: string; + url: string; + category?: string; +} + +function matchesSearchAndCategory( + entry: GatewayServerLike, + query: string, + category: string | null, +): boolean { + if (category && entry.category !== category) return false; + if (!query) return true; + return ( + entry.name.toLowerCase().includes(query) || + (entry.description ?? "").toLowerCase().includes(query) || + entry.url.toLowerCase().includes(query) + ); +} + +/** Home-screen filter: search over name/description/url plus category chip. */ +export function filterGatewayServers( + servers: McpGatewayServer[], + search: string, + category: string | null, +): McpGatewayServer[] { + const query = search.trim().toLowerCase(); + return servers.filter((server) => + matchesSearchAndCategory(server, query, category), + ); +} + +/** Same search/category filter, for catalog templates on the home screen. */ +export function filterCatalogTemplates( + templates: T[], + search: string, + category: string | null, +): T[] { + const query = search.trim().toLowerCase(); + return templates.filter((template) => + matchesSearchAndCategory(template, query, category), + ); +} + +/** Trailing-slash-insensitive URL identity for row/template matching. */ +export function normalizeGatewayServerUrl(url: string): string { + return url.trim().replace(/\/+$/, ""); +} + +/** + * The registry is sparse: a catalog template has a gateway row only once + * someone connected to it or an admin toggled it. "Recommended" templates are + * the active catalog entries with no row — matched neither by template id nor + * by URL (trailing-slash-insensitive) — shown as connect-only cards. + */ +export function recommendedCatalogTemplates< + T extends { id: string; url: string }, +>( + servers: Pick[], + templates: T[], +): T[] { + const rowTemplateIds = new Set(); + const rowUrls = new Set(); + for (const server of servers) { + if (server.template_id) rowTemplateIds.add(server.template_id); + rowUrls.add(normalizeGatewayServerUrl(server.url)); + } + return templates.filter( + (template) => + !rowTemplateIds.has(template.id) && + !rowUrls.has(normalizeGatewayServerUrl(template.url)), + ); +} + +export function countGatewayServersByCategory( + servers: McpGatewayServer[], +): Record { + const counts: Record = {}; + for (const server of servers) { + counts[server.category] = (counts[server.category] ?? 0) + 1; + } + return counts; +} + +/** + * Whether the current user can call this server without connecting first — + * every credential is the caller's own, so this is their connection's state. + */ +export function isConnectedForYou(server: McpGatewayServer): boolean { + return ( + !!server.your_connection && + getGatewayConnectionStatus(server.your_connection) === "connected" + ); +} + +export type GatewayConnectionStatus = + | "connected" + | "pending_oauth" + | "needs_reauth"; + +/** A persisted installation row is not necessarily a usable connection. */ +export function getGatewayConnectionStatus( + connection: Pick, +): GatewayConnectionStatus { + if (connection.needs_reauth) return "needs_reauth"; + if (connection.pending_oauth) return "pending_oauth"; + return "connected"; +} + +export type GatewayRailStatus = + | GatewayConnectionStatus + | "team_off" + | "revoked" + | "self_disabled"; + +/** + * Rail-row status: folds the switches the raw connection status can't see — + * the admin master switch, per-user revocation, and the caller's own enable + * toggle — so a connection that can't be used never reads as connected. The + * auth states only matter once all three switches are on. Null when the + * caller has no connection. + */ +export function getGatewayRailStatus( + server: Pick< + McpGatewayServer, + "is_team_enabled" | "is_revoked_for_you" | "your_connection" + >, +): GatewayRailStatus | null { + const connection = server.your_connection; + if (!connection) return null; + if (!server.is_team_enabled) return "team_off"; + if (server.is_revoked_for_you) return "revoked"; + if (!connection.is_enabled) return "self_disabled"; + return getGatewayConnectionStatus(connection); +} + +export type GatewayServerRemovalAction = + | "delete_for_everyone" + | "delete_for_you" + | "disconnect"; + +/** + * Admins remove custom servers from the team gateway. For members, a custom + * server they registered themselves is theirs to delete; catalog servers and + * custom servers registered by somebody else remain team entries, so removing + * the caller's installation is presented as disconnecting instead. + * + * The caller's identity must come in from the session user — + * `server.connections` is admin-only (empty for members), so it cannot + * identify a member caller. + */ +export function getGatewayServerRemovalAction( + server: McpGatewayServer, + isAdmin: boolean, + currentUserId: number | null, +): GatewayServerRemovalAction | null { + if (isAdmin && server.template_id === null) return "delete_for_everyone"; + + if (!server.your_connection) return null; + + const personallyAddedCustomServer = + server.template_id === null && + server.created_by !== null && + server.created_by.id === currentUserId; + + return personallyAddedCustomServer ? "delete_for_you" : "disconnect"; +} + +export type GatewayPolicyCounts = Record; + +export const AGENT_POLICY_STATES = [ + "approved", + "do_not_use", +] as const satisfies readonly McpApprovalState[]; + +export type AgentPolicyState = (typeof AGENT_POLICY_STATES)[number]; + +export function isAgentPolicyState( + state: McpApprovalState, +): state is AgentPolicyState { + return state !== "needs_approval"; +} + +/** + * Agents have no approval responder. A policy that would wait for approval is + * therefore unavailable to the agent, just like an explicit block. + */ +export function resolvePolicyStateForScope( + state: McpApprovalState, + scopeType: McpGatewayScopeType, +): McpApprovalState { + return scopeType === "agent" && state === "needs_approval" + ? "do_not_use" + : state; +} + +const POLICY_STRICTNESS: Record = { + approved: 0, + needs_approval: 1, + do_not_use: 2, +}; + +/** A scope may match the team ceiling or choose a more restrictive state. */ +export function isPolicyStateAllowedByCeiling( + state: McpApprovalState, + ceiling: McpApprovalState | null | undefined, +): boolean { + return ceiling === null || ceiling === undefined + ? true + : POLICY_STRICTNESS[state] >= POLICY_STRICTNESS[ceiling]; +} + +export function countPoliciesByState( + policies: McpResolvedToolPolicy[], + scopeType: McpGatewayScopeType = "member", +): GatewayPolicyCounts { + const counts: GatewayPolicyCounts = { + approved: 0, + needs_approval: 0, + do_not_use: 0, + }; + for (const policy of policies) { + counts[resolvePolicyStateForScope(policy.policy_state, scopeType)] += 1; + } + return counts; +} + +/** "2h ago" / "just now" for last-used and last-active timestamps. */ +export function formatAgo(timestamp: string | null): string | null { + if (!timestamp) return null; + const short = formatRelativeTimeShort(timestamp); + return short === "now" ? "just now" : `${short} ago`; +} + +/** Audit-table timestamp: "Today 09:58", "Yesterday 17:22", "Jul 15 09:12". */ +export function formatAuditTime(timestamp: string, now?: Date): string { + const date = new Date(timestamp); + const dayDiff = getLocalDayDiff(date, now); + const time = date.toLocaleTimeString(undefined, { + hour: "2-digit", + minute: "2-digit", + hour12: false, + }); + if (dayDiff <= 0) return `Today ${time}`; + if (dayDiff === 1) return `Yesterday ${time}`; + const day = date.toLocaleDateString(undefined, { + month: "short", + day: "numeric", + }); + return `${day} ${time}`; +} + +export const AUDIT_DECISION_LABELS: Record = { + auto: "Auto-approved", + approved: "Approved", + pending: "Awaiting approval", + blocked: "Blocked", +}; + +// Mirrors the backend's destructive-tool heuristic; only used to seed the +// per-tool defaults when sharing a server with an agent. +const DESTRUCTIVE_TOOL_RE = + /delete|update|post|write|create|run-migration|close|drop|send/; + +/** Default policy offered when granting an agent access to a tool. */ +export function defaultAgentGrantPolicy(toolName: string): AgentPolicyState { + return DESTRUCTIVE_TOOL_RE.test(toolName) ? "do_not_use" : "approved"; +} diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayToolDiscovery.test.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayToolDiscovery.test.ts new file mode 100644 index 000000000000..d90e8004c266 --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayToolDiscovery.test.ts @@ -0,0 +1,179 @@ +import type { + McpGatewayServer, + McpGatewayYourConnection, +} from "@posthog/api-client/posthog-client"; +import { describe, expect, it, vi } from "vitest"; +import { + discoverGatewayTools, + findGatewayServer, + shouldDiscoverGatewayTools, + usableInstallationId, +} from "./gatewayToolDiscovery"; + +function connection( + overrides: Partial = {}, +): McpGatewayYourConnection { + return { + installation_id: "inst-1", + is_enabled: true, + pending_oauth: false, + needs_reauth: false, + last_used_at: null, + ...overrides, + }; +} + +function server(overrides: Partial = {}): McpGatewayServer { + return { + id: "srv-1", + name: "Linear", + url: "https://mcp.linear.app/sse", + description: "", + category: "dev", + is_team_enabled: true, + icon_key: "", + docs_url: "", + template_id: null, + template_auth_type: null, + tool_count: 0, + connections: [], + your_connection: null, + agents: [], + revoked_user_ids: [], + is_revoked_for_you: false, + created_by: null, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + ...overrides, + }; +} + +function client(servers: McpGatewayServer[]) { + return { + getMcpGatewayServers: vi.fn().mockResolvedValue(servers), + refreshMcpInstallationTools: vi.fn().mockResolvedValue([]), + }; +} + +describe("findGatewayServer", () => { + const servers = [ + server({ id: "srv-1", template_id: "linear", url: "https://a.example" }), + server({ + id: "srv-2", + template_id: null, + url: "https://b.example/", + your_connection: connection({ installation_id: "inst-2" }), + }), + ]; + + it.each([ + ["by id", { serverId: "srv-2" }, "srv-2"], + ["by installation", { installationId: "inst-2" }, "srv-2"], + ["by template", { templateId: "linear" }, "srv-1"], + ["by url", { url: "https://b.example" }, "srv-2"], + [ + "by url ignoring a trailing slash", + { url: "https://a.example/" }, + "srv-1", + ], + [ + "falls back to url when the template misses", + { templateId: "unknown", url: "https://b.example" }, + "srv-2", + ], + ])("matches %s", (_label, match, expected) => { + expect(findGatewayServer(servers, match)?.id).toBe(expected); + }); + + it.each([ + ["nothing matches", { serverId: "missing" }], + ["no match criteria", {}], + ])("returns null when %s", (_label, match) => { + expect(findGatewayServer(servers, match)).toBeNull(); + }); +}); + +describe("usableInstallationId", () => { + it.each([ + ["a connected credential", connection(), "inst-1"], + ["a self-disabled connection", connection({ is_enabled: false }), "inst-1"], + ["a pending oauth connection", connection({ pending_oauth: true }), null], + ["a stale connection", connection({ needs_reauth: true }), null], + ["no connection", null, null], + ])("resolves %s", (_label, your_connection, expected) => { + expect(usableInstallationId(server({ your_connection }))).toBe(expected); + }); + + it("returns null without a server", () => { + expect(usableInstallationId(null)).toBeNull(); + }); +}); + +describe("shouldDiscoverGatewayTools", () => { + it.each([ + ["an empty catalog and a live connection", 0, connection(), true], + ["an already-populated catalog", 12, connection(), false], + ["no usable connection", 0, connection({ needs_reauth: true }), false], + ])("is %s -> %s", (_label, tool_count, your_connection, expected) => { + expect( + shouldDiscoverGatewayTools(server({ tool_count, your_connection })), + ).toBe(expected); + }); +}); + +describe("discoverGatewayTools", () => { + it("lists tools through the caller's fresh connection", async () => { + const api = client([server({ your_connection: connection() })]); + + const result = await discoverGatewayTools(api, { serverId: "srv-1" }); + + expect(api.refreshMcpInstallationTools).toHaveBeenCalledWith("inst-1"); + expect(result).toEqual({ + serverId: "srv-1", + installationId: "inst-1", + discovered: true, + }); + }); + + it("re-reads the registry so a just-created row is visible", async () => { + const api = client([ + server({ template_id: "linear", your_connection: connection() }), + ]); + + await discoverGatewayTools(api, { templateId: "linear" }); + + expect(api.getMcpGatewayServers).toHaveBeenCalledTimes(1); + }); + + it("uses a caller-supplied registry snapshot instead of re-reading", async () => { + const api = client([]); + const servers = [server({ your_connection: connection() })]; + + const result = await discoverGatewayTools( + api, + { serverId: "srv-1" }, + { servers }, + ); + + expect(api.getMcpGatewayServers).not.toHaveBeenCalled(); + expect(result.discovered).toBe(true); + }); + + it.each([ + ["no-server", [], { serverId: "missing" }], + ["no-connection", [server()], { serverId: "srv-1" }], + [ + "already-populated", + [server({ tool_count: 9, your_connection: connection() })], + { serverId: "srv-1" }, + ], + ])("skips with %s", async (skipped, servers, match) => { + const api = client(servers); + + const result = await discoverGatewayTools(api, match); + + expect(result.discovered).toBe(false); + expect(result.skipped).toBe(skipped); + expect(api.refreshMcpInstallationTools).not.toHaveBeenCalled(); + }); +}); diff --git a/products/desktop/packages/core/src/mcp-gateway/gatewayToolDiscovery.ts b/products/desktop/packages/core/src/mcp-gateway/gatewayToolDiscovery.ts new file mode 100644 index 000000000000..9f591dce670b --- /dev/null +++ b/products/desktop/packages/core/src/mcp-gateway/gatewayToolDiscovery.ts @@ -0,0 +1,138 @@ +import type { McpGatewayServer } from "@posthog/api-client/posthog-client"; +import { normalizeGatewayServerUrl } from "./gatewayServers"; + +/** + * A gateway server's tool catalog only exists once something lists it from the + * upstream server through a caller's credential. Connecting stores the + * credential but discovers nothing, so without this the registry keeps a row + * with zero tools until an admin hits the manual refresh button. + */ +export interface GatewayToolDiscoveryClient { + getMcpGatewayServers(): Promise; + refreshMcpInstallationTools(installationId: string): Promise; +} + +/** How to find the just-connected server in the re-read registry. */ +export interface GatewayServerMatch { + serverId?: string | null; + /** The caller's own installation, for flows that only know the credential. */ + installationId?: string | null; + templateId?: string | null; + url?: string | null; +} + +export type GatewayToolDiscoverySkip = + | "no-server" + | "no-connection" + | "already-populated"; + +export interface GatewayToolDiscoveryResult { + serverId: string | null; + installationId: string | null; + discovered: boolean; + skipped?: GatewayToolDiscoverySkip; +} + +/** + * Match a registry row by id, then installation, then template, then URL. The + * add-server flow only knows the URL it submitted; connect-from-catalog only + * knows the template id; reconnect only knows the installation. + */ +export function findGatewayServer( + servers: McpGatewayServer[], + match: GatewayServerMatch, +): McpGatewayServer | null { + if (match.serverId) { + return servers.find((server) => server.id === match.serverId) ?? null; + } + if (match.installationId) { + const byInstallation = servers.find( + (server) => + server.your_connection?.installation_id === match.installationId, + ); + if (byInstallation) return byInstallation; + } + if (match.templateId) { + const byTemplate = servers.find( + (server) => server.template_id === match.templateId, + ); + if (byTemplate) return byTemplate; + } + if (match.url) { + const target = normalizeGatewayServerUrl(match.url); + return ( + servers.find( + (server) => normalizeGatewayServerUrl(server.url) === target, + ) ?? null + ); + } + return null; +} + +/** + * The caller's own installation, when it can actually reach the server. + * A self-disabled connection still holds a usable credential; one that is + * mid-OAuth or needs reauth does not. + */ +export function usableInstallationId( + server: McpGatewayServer | null, +): string | null { + const connection = server?.your_connection; + if (!connection) return null; + if (connection.pending_oauth || connection.needs_reauth) return null; + return connection.installation_id; +} + +/** + * Discover only when the team has no catalog yet. A populated `tool_count` + * means someone already listed the tools, and re-listing on every connect + * would hit the upstream server for nothing. + */ +export function shouldDiscoverGatewayTools( + server: McpGatewayServer | null, +): boolean { + if (!server) return false; + if (server.tool_count > 0) return false; + return usableInstallationId(server) !== null; +} + +/** + * Re-read the registry after a connect and, if the server still has no tools, + * list them through the caller's fresh credential. Callers invalidate the + * returned `serverId`'s tool queries when `discovered` is true. + */ +export async function discoverGatewayTools( + client: GatewayToolDiscoveryClient, + match: GatewayServerMatch, + options: { servers?: McpGatewayServer[] } = {}, +): Promise { + const servers = options.servers ?? (await client.getMcpGatewayServers()); + const server = findGatewayServer(servers, match); + if (!server) { + return { + serverId: null, + installationId: null, + discovered: false, + skipped: "no-server", + }; + } + const installationId = usableInstallationId(server); + if (!installationId) { + return { + serverId: server.id, + installationId: null, + discovered: false, + skipped: "no-connection", + }; + } + if (server.tool_count > 0) { + return { + serverId: server.id, + installationId, + discovered: false, + skipped: "already-populated", + }; + } + await client.refreshMcpInstallationTools(installationId); + return { serverId: server.id, installationId, discovered: true }; +} diff --git a/products/desktop/packages/core/src/pi-runtime/piSessionController.test.ts b/products/desktop/packages/core/src/pi-runtime/piSessionController.test.ts index 897e1799eef3..37df1cae8eb3 100644 --- a/products/desktop/packages/core/src/pi-runtime/piSessionController.test.ts +++ b/products/desktop/packages/core/src/pi-runtime/piSessionController.test.ts @@ -819,6 +819,43 @@ describe("PiSessionController", () => { expect(resumedSession.client.prompt).toHaveBeenCalledWith("continue"); }); + it("applies deferred Pi config before the first resumed prompt", async () => { + const terminalSession = { + ...createSession(), + resumeRequired: true, + taskRunId: "run-1", + }; + const resumedSession = createSession(); + const provider = { + get: vi + .fn() + .mockResolvedValueOnce(terminalSession) + .mockResolvedValue(resumedSession), + } as PiSessionProvider; + const resumeCloudPiRun = vi.fn(async () => ({ id: "run-1" })); + const controller = new PiSessionController(provider, { + resumeCloudPiRun, + } as unknown as TaskService); + + await controller.connect("task-1"); + await controller.submit("task-1", "continue", false, "steer", { + model: { provider: "posthog", id: "gpt-5.6-terra" }, + thinkingLevel: "high", + }); + + expect(resumedSession.client.setModel).toHaveBeenCalledWith( + "posthog", + "gpt-5.6-terra", + ); + expect(resumedSession.client.setThinkingLevel).toHaveBeenCalledWith("high"); + expect(resumedSession.client.prompt).toHaveBeenCalledWith("continue"); + expect( + vi.mocked(resumedSession.client.setModel).mock.invocationCallOrder[0], + ).toBeLessThan( + vi.mocked(resumedSession.client.prompt).mock.invocationCallOrder[0], + ); + }); + it("resumes and retries a message when the prior sandbox is gone", async () => { const staleSession = { ...createSession(), diff --git a/products/desktop/packages/core/src/pi-runtime/piSessionController.ts b/products/desktop/packages/core/src/pi-runtime/piSessionController.ts index f2b202350d9d..f86aedb30244 100644 --- a/products/desktop/packages/core/src/pi-runtime/piSessionController.ts +++ b/products/desktop/packages/core/src/pi-runtime/piSessionController.ts @@ -35,6 +35,11 @@ export type { export type PiModelSelection = Pick; +export interface PiDeferredConfig { + model?: PiModelSelection; + thinkingLevel?: PiThinkingLevel; +} + export const PI_SESSION_PROVIDER = Symbol.for("posthog.pi.sessionProvider"); export const LOCAL_PI_SESSION_FACTORY = Symbol.for( "posthog.pi.localSessionFactory", @@ -301,6 +306,7 @@ export class PiSessionController { text: string, isStreaming: boolean, messagingMode: PiMessagingMode, + deferredConfig?: PiDeferredConfig, ): Promise { const message = text.trim(); const action = this.getSubmitAction(message, isStreaming, messagingMode); @@ -382,6 +388,7 @@ export class PiSessionController { try { const session = await this.getWritablePiSession(taskId); + await this.applyDeferredConfig(session, deferredConfig); this.markTurnPending(taskId); if (session.sendUserMessage && messageId) { const taskRunId = this.taskRunIds.get(taskId); @@ -1088,6 +1095,22 @@ export class PiSessionController { }); } + private async applyDeferredConfig( + session: PiSession, + config: PiDeferredConfig | undefined, + ): Promise { + if (!config) { + return; + } + + if (config.model) { + await session.client.setModel(config.model.provider, config.model.id); + } + if (config.thinkingLevel) { + await session.client.setThinkingLevel(config.thinkingLevel); + } + } + private async refreshStatus(taskId: string): Promise { const session = await this.getPiSession(taskId); const status = await session.client.getState(); diff --git a/products/desktop/packages/core/src/task-detail/taskCreationSaga.test.ts b/products/desktop/packages/core/src/task-detail/taskCreationSaga.test.ts index 3fa05dc72a3b..1d3849110da0 100644 --- a/products/desktop/packages/core/src/task-detail/taskCreationSaga.test.ts +++ b/products/desktop/packages/core/src/task-detail/taskCreationSaga.test.ts @@ -453,8 +453,8 @@ describe("TaskCreationSaga", () => { branch: "main", adapter: undefined, piRuntime: true, - model: undefined, - reasoningLevel: undefined, + model: "gpt-5.4", + reasoningLevel: "high", initialPermissionMode: undefined, }), ); diff --git a/products/desktop/packages/core/src/task-detail/taskCreationSaga.ts b/products/desktop/packages/core/src/task-detail/taskCreationSaga.ts index 461c39fca44e..da3f55a9caec 100644 --- a/products/desktop/packages/core/src/task-detail/taskCreationSaga.ts +++ b/products/desktop/packages/core/src/task-detail/taskCreationSaga.ts @@ -408,8 +408,8 @@ export class TaskCreationSaga extends Saga< branch, adapter: cloudAdapter, ...(isPiRuntime ? { piRuntime: true } : {}), - model: isPiRuntime ? undefined : input.model, - reasoningLevel: isPiRuntime ? undefined : input.reasoningLevel, + model: input.model, + reasoningLevel: input.reasoningLevel, contextWindow: isPiRuntime ? undefined : input.contextWindow, fastMode: isPiRuntime ? undefined : input.fastMode, sandboxEnvironmentId: input.sandboxEnvironmentId, diff --git a/products/desktop/packages/harness/package.json b/products/desktop/packages/harness/package.json index b0f39fd3c7a8..a158c46ba3e6 100644 --- a/products/desktop/packages/harness/package.json +++ b/products/desktop/packages/harness/package.json @@ -97,6 +97,7 @@ "@posthog/shared": "workspace:*", "lru-cache": "^11.1.0", "turndown": "^7.2.4", + "typebox": "1.3.7", "zod": "^4.2.0" }, "devDependencies": { diff --git a/products/desktop/packages/harness/src/extensions/mcp/schema.test.ts b/products/desktop/packages/harness/src/extensions/mcp/schema.test.ts index c96850e6ad05..bd0fbe574081 100644 --- a/products/desktop/packages/harness/src/extensions/mcp/schema.test.ts +++ b/products/desktop/packages/harness/src/extensions/mcp/schema.test.ts @@ -1,3 +1,4 @@ +import { Compile } from "typebox/compile"; import { describe, expect, it } from "vitest"; import { convertJsonSchemaToTypebox } from "./schema"; @@ -107,6 +108,18 @@ describe("convertJsonSchemaToTypebox", () => { expect(result.anyOf?.[1]).toMatchObject({ type: "null" }); }); + it("validates nullable arrays", () => { + const schema = convertJsonSchemaToTypebox({ + type: ["array", "null"], + items: { type: "string" }, + }); + const check = Compile(schema); + + expect(check.Check(null)).toBe(true); + expect(check.Check(["value"])).toBe(true); + expect(check.Check([1])).toBe(false); + }); + it.each([["oneOf"], ["anyOf"]])("converts %s to a union", (key) => { const result = convert({ [key]: [{ type: "string" }, { type: "number" }], diff --git a/products/desktop/packages/agent/src/pi/model-catalog.test.ts b/products/desktop/packages/harness/src/extensions/posthog-provider/model-catalog.test.ts similarity index 100% rename from products/desktop/packages/agent/src/pi/model-catalog.test.ts rename to products/desktop/packages/harness/src/extensions/posthog-provider/model-catalog.test.ts diff --git a/products/desktop/packages/agent/src/pi/model-catalog.ts b/products/desktop/packages/harness/src/extensions/posthog-provider/model-catalog.ts similarity index 85% rename from products/desktop/packages/agent/src/pi/model-catalog.ts rename to products/desktop/packages/harness/src/extensions/posthog-provider/model-catalog.ts index 96b6bb45d592..9642b0924921 100644 --- a/products/desktop/packages/agent/src/pi/model-catalog.ts +++ b/products/desktop/packages/harness/src/extensions/posthog-provider/model-catalog.ts @@ -2,20 +2,22 @@ import { getSupportedThinkingLevels, type ModelThinkingLevel, } from "@earendil-works/pi-ai"; +import type { ModelInfo } from "@earendil-works/pi-coding-agent"; +import type { CloudRegion } from "@posthog/shared"; import { fetchPosthogGatewayModels, type GatewayModel, resolveModelConfigsFromGatewayModels, -} from "@posthog/harness/extensions/posthog-provider/models"; -import type { CloudRegion } from "@posthog/shared"; +} from "./models"; -export interface PiModelCatalogEntry { +export type PiModelCatalogEntry = Omit< + Pick, + "provider" +> & { provider: "posthog"; - id: string; name: string; - contextWindow: number; thinkingLevels: ModelThinkingLevel[]; -} +}; export function resolvePosthogPiModelCatalog( gatewayModels: GatewayModel[], diff --git a/products/desktop/packages/harness/src/runtime.test.ts b/products/desktop/packages/harness/src/runtime.test.ts index 6314e26b35ee..d127d073ecc0 100644 --- a/products/desktop/packages/harness/src/runtime.test.ts +++ b/products/desktop/packages/harness/src/runtime.test.ts @@ -111,7 +111,7 @@ describe("createHarnessRuntime", () => { posthogOAuthCredentials: { access: "access-token", refresh: "refresh-token", - expires: Date.now() + 60_000, + expires: Date.now() + 10 * 60_000, region: "us", }, sessionManager: pi.SessionManager.inMemory(cwd), @@ -150,7 +150,7 @@ describe("createHarnessRuntime", () => { posthogOAuthCredentials: { access: "access-token", refresh: "refresh-token", - expires: Date.now() + 60_000, + expires: Date.now() + 10 * 60_000, region: "us", }, sessionManager: pi.SessionManager.inMemory(cwd), diff --git a/products/desktop/packages/harness/tsup.config.ts b/products/desktop/packages/harness/tsup.config.ts index fe4556659834..0ebc718d44f9 100644 --- a/products/desktop/packages/harness/tsup.config.ts +++ b/products/desktop/packages/harness/tsup.config.ts @@ -13,6 +13,7 @@ export default defineConfig({ "src/extensions/posthog-provider/index.ts", "src/extensions/posthog-provider/provider.ts", "src/extensions/posthog-provider/models.ts", + "src/extensions/posthog-provider/model-catalog.ts", "src/extensions/posthog-provider/oauth.ts", "src/extensions/posthog-provider/gateway.ts", "src/extensions/posthog-provider/gateway-auth.ts", diff --git a/products/desktop/packages/shared/src/analytics-events.ts b/products/desktop/packages/shared/src/analytics-events.ts index eea1430b6fbb..82bdd241b73a 100644 --- a/products/desktop/packages/shared/src/analytics-events.ts +++ b/products/desktop/packages/shared/src/analytics-events.ts @@ -327,6 +327,15 @@ export interface CloudStreamDisconnectedProperties { was_bootstrapping: boolean; } +export interface CloudStreamIdleTimeoutProperties { + task_id: string; + run_id: string; + team_id: number; + idle_timeout_ms: number; + bytes_received: number; + events_received: number; +} + // Permission events export interface PermissionRespondedProperties { task_id: string; @@ -1378,6 +1387,7 @@ export const ANALYTICS_EVENTS = { TASK_CREATION_FAILED: "Task creation failed", AGENT_SESSION_ERROR: "Agent session error", CLOUD_STREAM_DISCONNECTED: "Cloud stream disconnected", + CLOUD_STREAM_IDLE_TIMEOUT: "Cloud stream idle timeout", // Inbox events INBOX_VIEWED: "Inbox viewed", @@ -1556,6 +1566,7 @@ export type EventPropertyMap = { [ANALYTICS_EVENTS.TASK_CREATION_FAILED]: TaskCreationFailedProperties; [ANALYTICS_EVENTS.AGENT_SESSION_ERROR]: AgentSessionErrorProperties; [ANALYTICS_EVENTS.CLOUD_STREAM_DISCONNECTED]: CloudStreamDisconnectedProperties; + [ANALYTICS_EVENTS.CLOUD_STREAM_IDLE_TIMEOUT]: CloudStreamIdleTimeoutProperties; // Inbox events [ANALYTICS_EVENTS.INBOX_VIEWED]: InboxViewedProperties; diff --git a/products/desktop/packages/shared/src/flags.ts b/products/desktop/packages/shared/src/flags.ts index 3a4ee8118b4e..fe5834a13d10 100644 --- a/products/desktop/packages/shared/src/flags.ts +++ b/products/desktop/packages/shared/src/flags.ts @@ -31,5 +31,11 @@ export const FAST_MODE_FLAG = "posthog-desktop-fast-mode"; export const SPOKEN_NARRATION_FLAG = "posthog-code-spoken-narration"; // Gates importing and relaying local MCP servers into cloud task runs. export const LOCAL_MCP_IMPORT_FLAG = "posthog-code-local-mcp-import"; +/** + * Team MCP gateway (shared credentials, per-scope tool policies, agent + * service accounts, audit log) replacing the per-user MCP marketplace. + * Owned by the backend rollout in posthog/posthog — same flag key there. + */ +export const MCP_GATEWAY_FLAG = "mcp-gateway"; /** Per-task estimated cost readout in the context usage indicator. */ export const TASK_COST_FLAG = "posthog-code-task-cost"; diff --git a/products/desktop/packages/shared/src/index.ts b/products/desktop/packages/shared/src/index.ts index 1648fb76d0f9..56ce3353c34c 100644 --- a/products/desktop/packages/shared/src/index.ts +++ b/products/desktop/packages/shared/src/index.ts @@ -359,9 +359,11 @@ export type { } from "./task-creation-domain"; export { formatClockTime, + formatDaySeparatorLabel, formatRelativeTimeLong, formatRelativeTimeShort, getLocalDayDiff, + getLocalDayKey, getRelativeDateGroup, } from "./time"; export { diff --git a/products/desktop/packages/shared/src/time.test.ts b/products/desktop/packages/shared/src/time.test.ts index d0a69f92ece2..f76961a0a85d 100644 --- a/products/desktop/packages/shared/src/time.test.ts +++ b/products/desktop/packages/shared/src/time.test.ts @@ -1,9 +1,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { formatClockTime, + formatDaySeparatorLabel, formatRelativeTimeLong, formatRelativeTimeShort, getLocalDayDiff, + getLocalDayKey, getRelativeDateGroup, } from "./time"; @@ -124,3 +126,37 @@ describe("getRelativeDateGroup", () => { expect(getRelativeDateGroup(NOW - 40 * DAY)).toBe("Earlier"); }); }); + +describe("getLocalDayKey", () => { + it("gives two times on the same local day one key", () => { + expect(getLocalDayKey(new Date(2026, 5, 15, 0, 1))).toBe( + getLocalDayKey(new Date(2026, 5, 15, 23, 59)), + ); + }); + + it("separates adjacent days", () => { + expect(getLocalDayKey(new Date(2026, 5, 15))).not.toBe( + getLocalDayKey(new Date(2026, 5, 16)), + ); + }); +}); + +describe("formatDaySeparatorLabel", () => { + const now = new Date(2026, 5, 15, 12); + + it.each([ + ["today", new Date(2026, 5, 15, 9), "Today"], + ["yesterday", new Date(2026, 5, 14, 9), "Yesterday"], + // Within the week the weekday alone is unambiguous. + ["earlier this week", new Date(2026, 5, 11), "Thursday 11th"], + // Past a week it needs the month, and past a year the year too. + ["last month", new Date(2026, 4, 20), "Wednesday, May 20th"], + ["last year", new Date(2025, 11, 3), "Wednesday, December 3rd, 2025"], + ])("labels %s", (_case, date: Date, expected) => { + expect(formatDaySeparatorLabel(date, now)).toBe(expected); + }); + + it("labels a future timestamp as today rather than counting backwards", () => { + expect(formatDaySeparatorLabel(new Date(2026, 5, 16), now)).toBe("Today"); + }); +}); diff --git a/products/desktop/packages/shared/src/time.ts b/products/desktop/packages/shared/src/time.ts index 3cfd21ba6508..08c3e822aee3 100644 --- a/products/desktop/packages/shared/src/time.ts +++ b/products/desktop/packages/shared/src/time.ts @@ -75,6 +75,48 @@ export function getLocalDayDiff( return Math.round((startOfDay(now) - startOfDay(date)) / 86_400_000); } +/** + * Local calendar-day identity, for deciding where a day separator goes. Two + * timestamps on the same day share a key regardless of time, and the key is + * built from local getters (not the UTC ISO) so the split lands on the viewer's + * midnight. + */ +export function getLocalDayKey(timestamp: number | string | Date): string { + const date = new Date(timestamp); + return `${date.getFullYear()}-${date.getMonth()}-${date.getDate()}`; +} + +function ordinal(n: number): string { + const suffix = ["th", "st", "nd", "rd"]; + const rem = n % 100; + return `${n}${suffix[(rem - 20) % 10] ?? suffix[rem] ?? suffix[0]}`; +} + +/** + * A day separator's label: "Today" / "Yesterday" for the recent days, then a + * weekday + ordinal ("Monday 5th") within the week, adding the month (and the + * year when it differs) further back so older separators stay unambiguous. + * + * Shared by the space feed and the space sidebar's recents, so the same day is + * never named two different ways in one window. + */ +export function formatDaySeparatorLabel( + timestamp: number | string | Date, + now: Date = new Date(), +): string { + const date = new Date(timestamp); + const days = getLocalDayDiff(date, now); + if (days <= 0) return "Today"; + if (days === 1) return "Yesterday"; + const weekday = date.toLocaleDateString(undefined, { weekday: "long" }); + const day = ordinal(date.getDate()); + if (days < 7) return `${weekday} ${day}`; + const month = date.toLocaleDateString(undefined, { month: "long" }); + const year = + date.getFullYear() === now.getFullYear() ? "" : `, ${date.getFullYear()}`; + return `${weekday}, ${month} ${day}${year}`; +} + export function getRelativeDateGroup( timestamp: number | string, ): string | null { diff --git a/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx b/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx index c8a72cc02058..ebf71c7ceb3e 100644 --- a/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx +++ b/products/desktop/packages/ui/src/features/browser-tabs/BrowserTabStrip.tsx @@ -166,8 +166,11 @@ export function BrowserTabStrip() { // decides where a task/blank tab navigates. const inChannels = pathname.startsWith("/website"); // Top-level app pages (Inbox, Agents, Skills, MCP servers, Command Center) - // are tab targets too. useAppView normalizes both the /code routes and - // their /website mirrors to the same view.type, so a tab survives either space. + // are tab targets too. useAppView normalizes both the /code routes and their + // /website mirrors to the same view.type, so a tab survives either space. A + // top-level route that ISN'T here falls through to `task-input`, and the + // strip then reconciles the location against the wrong tab and navigates + // straight back off the page. const view = useAppView(); const routeAppView: AppView | null = isAppView(view.type) ? view.type : null; diff --git a/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.test.tsx index b6ea0cb52dfa..8d0e777678b2 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.test.tsx @@ -128,4 +128,37 @@ describe("ActivityTimeline", () => { expect(screen.getByText("Saved workspace context")).toBeVisible(); expect(useThreadNavigationStore.getState().scrollRequests).toEqual({}); }); + + it("hides injected custom instructions from conversation previews", () => { + renderTimeline(true, [ + { + type: "user_message", + id: "custom-instructions-message", + content: + "Review this PR\n\n\nThe user has saved custom instructions that apply to all of their tasks. Follow them.\n\nNever update an existing PR description.\n", + timestamp: Date.parse("2026-07-17T09:05:00Z"), + }, + ]); + + expect(screen.getByText("Review this PR")).toBeInTheDocument(); + expect(screen.queryByText(/user_custom_instructions/)).toBeNull(); + expect( + screen.queryByText("Never update an existing PR description."), + ).toBeNull(); + }); + + it("shows user-authored custom-instruction tag examples", () => { + renderTimeline(true, [ + { + type: "user_message", + id: "literal-custom-instructions-message", + content: + "Render this example: be terse", + timestamp: Date.parse("2026-07-17T09:05:00Z"), + }, + ]); + + expect(screen.getByText(/user_custom_instructions/)).toBeInTheDocument(); + expect(screen.getByText(/be terse/)).toBeInTheDocument(); + }); }); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.tsx b/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.tsx index 94915cc36b5b..c899b0bbd259 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ActivityTimeline.tsx @@ -34,6 +34,7 @@ import { ThreadTimestamp } from "@posthog/ui/features/canvas/components/ThreadTi import { userDisplayName } from "@posthog/ui/features/canvas/utils/userDisplay"; import type { buildConversationItems } from "@posthog/ui/features/sessions/components/buildConversationItems"; import { extractChannelContext } from "@posthog/ui/features/sessions/components/session-update/channelContext"; +import { extractCustomInstructions } from "@posthog/ui/features/sessions/components/session-update/customInstructions"; import { useThreadNavigationStore } from "@posthog/ui/features/sessions/threadNavigationStore"; import { Fragment, type KeyboardEvent, type ReactNode, useMemo } from "react"; @@ -83,7 +84,12 @@ function UserMessageRow({ () => extractChannelContext(content), [content], ); - const displayContent = channelContext?.stripped ?? content; + const afterChannelContext = channelContext?.stripped ?? content; + const customInstructions = useMemo( + () => extractCustomInstructions(afterChannelContext), + [afterChannelContext], + ); + const displayContent = customInstructions?.stripped ?? afterChannelContext; // The row itself is the hit target. `ThreadItem` renders an
, which a // + ); } @@ -54,14 +56,20 @@ export function ChannelBackRow({ channelId }: { channelId: string }) { const { channels, isLoading } = useChannels(); const current = channels.find((c) => c.id === channelId); const showStar = current != null && current.name !== PERSONAL_CHANNEL_NAME; + const glyph = channelGlyph(current?.name, { + size: 14, + space: spacesLayout, + className: "text-muted-foreground", + }); return (
{ track(ANALYTICS_EVENTS.CHANNEL_ACTION, { @@ -71,25 +79,26 @@ export function ChannelBackRow({ channelId }: { channelId: string }) { }); showChannelList(); }} - // Fixed height with an unconditional star well: sized off its - // contents, a starrable channel ran 4px taller than #me and - // everything below shifted on switch. No border — it's a row in - // the sidebar like the ones under it, not a control sitting on - // top. - className="flex h-8 w-full items-center gap-1.5 rounded-md px-2 text-left transition-colors hover:bg-fill-hover" + // Quill's own height and radius, so this reads as one of the rows + // under it rather than a control sitting on top. The star well is + // unconditional (see the reserved span below): sized off its + // contents, a starrable channel ran taller than #me and everything + // below shifted on switch. + className="w-full gap-1.5 text-left" > - - {channelGlyph(current?.name, { - size: 14, - space: spacesLayout, - className: "text-muted-foreground", - })} - + {/* Only #me still has a glyph under the layout, and its well is + drawn only when there's something in it — an empty 16px column + in front of every other space's name is worse than the name + starting where the caret leaves off. */} + {glyph && ( + + {glyph} + + )} {current ? ( current.name @@ -102,7 +111,7 @@ export function ChannelBackRow({ channelId }: { channelId: string }) { )} - + } /> Back to spaces diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelFeedView.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelFeedView.tsx index ef985d5e6ecc..c0cc5547e1a3 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelFeedView.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelFeedView.tsx @@ -18,8 +18,6 @@ import { Badge, Card, CardContent, - ChatMarker, - ChatMarkerContent, ChatMessageScroller, ChatMessageScrollerButton, ChatMessageScrollerContent, @@ -42,7 +40,7 @@ import { ThreadItemTimestamp, useChatMessageScroller, } from "@posthog/quill"; -import { formatRelativeTimeShort, getLocalDayDiff } from "@posthog/shared"; +import { formatRelativeTimeShort } from "@posthog/shared"; import type { Task, TaskRunStatus, @@ -65,7 +63,6 @@ import { useInView } from "@posthog/ui/primitives/hooks/useInView"; import { Text } from "@radix-ui/themes"; import { Link } from "@tanstack/react-router"; import { - Fragment, memo, type ReactNode, useCallback, @@ -103,37 +100,6 @@ function statusBadge(status: TaskRunStatus) { ); } -// Local calendar-day identity, so tasks created on the same day share a heading -// regardless of time. Uses local getters (not the UTC ISO) so the split lands -// on the viewer's midnight. -function dayKey(iso: string): string { - const d = new Date(iso); - return `${d.getFullYear()}-${d.getMonth()}-${d.getDate()}`; -} - -function ordinal(n: number): string { - const suffix = ["th", "st", "nd", "rd"]; - const rem = n % 100; - return `${n}${suffix[(rem - 20) % 10] ?? suffix[rem] ?? suffix[0]}`; -} - -// The day-separator label: "Today" / "Yesterday" for the recent days, then a -// weekday + ordinal ("Monday 5th") within the week, adding the month (and the -// year when it differs) further back so older separators stay unambiguous. -function dayLabel(iso: string, now: Date): string { - const date = new Date(iso); - const days = getLocalDayDiff(date, now); - if (days <= 0) return "Today"; - if (days === 1) return "Yesterday"; - const weekday = date.toLocaleDateString(undefined, { weekday: "long" }); - const day = ordinal(date.getDate()); - if (days < 7) return `${weekday} ${day}`; - const month = date.toLocaleDateString(undefined, { month: "long" }); - const year = - date.getFullYear() === now.getFullYear() ? "" : `, ${date.getFullYear()}`; - return `${weekday}, ${month} ${day}${year}`; -} - interface TaskStatusDisplay { // The run/environment badge ("Local", "Completed", "In progress", …). base: ReactNode; @@ -861,33 +827,19 @@ export function ChannelFeedView({ selector, which hangs over the end of the feed. */} {intro} - {entries.map((entry, index) => { - const previous = entries[index - 1]; - const showDayMarker = - !previous || - dayKey(previous.createdAt) !== dayKey(entry.createdAt); - return ( - - {showDayMarker && ( - - - {dayLabel(entry.createdAt, now)} - - - )} - {entry.kind === "task" ? ( - - ) : ( - - )} - - ); - })} + {entries.map((entry) => + entry.kind === "task" ? ( + + ) : ( + + ), + )} {pending.map((p) => ( ({ status: null as TaskStatusInput | null })); +vi.mock("@posthog/ui/features/canvas/hooks/useChannelTaskStatus", () => ({ + useChannelTaskStatus: () => mocks.status, +})); +// The row menu's spaces list and filing mutation are tRPC-backed. +vi.mock("@posthog/ui/features/canvas/hooks/useChannels", () => ({ + useChannels: () => ({ channels: [{ id: "channel-1", name: "code" }] }), +})); +vi.mock("@posthog/ui/features/canvas/hooks/useFileTaskToChannel", () => ({ + useFileTaskToChannel: () => vi.fn(), +})); +vi.mock("@posthog/ui/features/feature-flags/useFeatureFlag", () => ({ + useFeatureFlag: () => true, +})); + +import { usePendingCanvasDeleteStore } from "@posthog/ui/features/canvas/stores/pendingCanvasDeleteStore"; import { ChannelItemRow } from "./ChannelItemRow"; const actions = { open: () => {}, togglePin: () => {}, archive: () => {}, + remove: () => {}, }; function item(overrides: Partial = {}): ChannelItemModel { @@ -24,6 +47,7 @@ function item(overrides: Partial = {}): ChannelItemModel { authorName: null, authorUuid: "user-uuid", templateId: null, + task: null, ...overrides, }; } @@ -36,21 +60,102 @@ function renderRow(model: ChannelItemModel) { ); } +beforeEach(() => { + mocks.status = null; + usePendingCanvasDeleteStore.setState({ pending: {} }); +}); + describe("ChannelItemRow", () => { + // The dot vocabulary in one table: what the row's leading mark says for each + // state a task can be in. Only the states a reader can act on get a voice — + // run mechanics (queued, failed) resolve to "working" or "something to read". it.each([ - ["queued" as const, true], - ["in_progress" as const, true], - ["not_started" as const, false], - ["completed" as const, false], - ["failed" as const, false], - ["cancelled" as const, false], - ])("marks %s as running: %s", (rawStatus: TaskRunStatus, running) => { - renderRow(item({ rawStatus })); - - expect(!!screen.queryByRole("img", { name: "Running" })).toBe(running); + [ + "a permission prompt", + { needsPermission: true }, + "Needs permission — blocked on you", + ], + ["a streaming agent", { isGenerating: true }, "Working"], + [ + // The run says in_progress, but nothing is streaming: a local run never + // gets a terminal status written, and the cloud one holds in_progress past + // the agent. Live, but not moving — the still dot, not the spinner. + "a run claiming progress with nothing in flight", + { taskRunStatus: "in_progress" as const }, + "Pending — no work in flight", + ], + [ + "a queued cloud run", + { taskRunStatus: "queued" as const }, + "Pending — no work in flight", + ], + [ + "a broken run with unseen output", + { taskRunStatus: "failed" as const, isUnread: true }, + "Unread — something to read", + ], + ["a suspended task", { isSuspended: true }, "Suspended — parked"], + [ + // The cloud workflow holds a run at in_progress while it babysits CI after + // opening the PR; under a merge queue that wait can outlast the agent by + // hours, so the PR's existence has to win over the run's claim. + "a run still babysitting CI behind an open PR", + { taskRunStatus: "in_progress" as const, prState: "open" as const }, + "All caught up", + ], + [ + "a run whose PR url is known but state isn't", + { + taskRunStatus: "in_progress" as const, + prUrl: "https://github.com/PostHog/code/pull/1", + }, + "All caught up", + ], + [ + // A live local session is the agent typing right now, which no PR overrides. + "a streaming agent that already has a PR", + { isGenerating: true, prState: "open" as const }, + "Working", + ], + [ + "a merged PR", + { prState: "merged" as const }, + // PR state lives on the badge, so the dot stays quiet. + "All caught up", + ], + ["an idle task", {}, "All caught up"], + ])("labels %s", (_case, status: TaskStatusInput, label) => { + mocks.status = status; + + renderRow(item()); + + expect(screen.getByRole("img", { name: label })).not.toBeNull(); }); - it("leaves a canvas, which has no run to wait on, static", () => { + it("badges a PR it can see the url of but not the state of", () => { + mocks.status = { + workspaceMode: "cloud", + prUrl: "https://github.com/PostHog/code/pull/1", + }; + + renderRow(item()); + + // Uncoloured, because colour is a verdict — but present, because a task that + // opened a PR must not look like it did nothing. + expect(screen.getByRole("img", { name: "Pull request" })).not.toBeNull(); + }); + + it("shows a task's badges instead of its timestamp", () => { + mocks.status = { workspaceMode: "cloud", prState: "merged" }; + + renderRow(item()); + + expect(screen.getByRole("img", { name: "Cloud" })).not.toBeNull(); + expect(screen.getByRole("img", { name: "Merged" })).not.toBeNull(); + expect(screen.queryByText(formatRelativeTimeShort(item().ts))).toBeNull(); + }); + + it("renders a canvas like a quiet task with its glyph in the badge stack", () => { renderRow( item({ key: "canvas:canvas-1", @@ -61,36 +166,171 @@ describe("ChannelItemRow", () => { }), ); - expect(screen.queryByRole("img", { name: "Running" })).toBeNull(); + expect(screen.getByRole("img", { name: "All caught up" })).not.toBeNull(); + expect(screen.getByRole("img", { name: "Canvas" })).not.toBeNull(); + expect(screen.queryByText(formatRelativeTimeShort(item().ts))).toBeNull(); + }); + + it("marks a pinned row with the pin badge, alongside its status badges", () => { + mocks.status = { workspaceMode: "cloud" }; + + renderRow(item({ pinned: true })); + + expect(screen.getByRole("img", { name: "Pinned" })).not.toBeNull(); + expect(screen.getByRole("img", { name: "Cloud" })).not.toBeNull(); }); - // The point of the shimmer over a spinner: a running task still looks like a - // task, so the list stays scannable by kind while work is in flight. - it("keeps the item's own glyph while running", () => { - renderRow(item({ rawStatus: "in_progress" })); + it("leaves an unpinned row without one", () => { + renderRow(item()); - const running = screen.getByRole("img", { name: "Running" }); - expect(running).toHaveClass("ph-shimmer"); - // The glyph is wrapped, not replaced — no spinner swapped in its place. - expect(running.querySelector("svg")).not.toBeNull(); + expect(screen.queryByRole("img", { name: "Pinned" })).toBeNull(); }); - it("opens the task context menu from the row", () => { - const onContextMenu = vi.fn(); + // The hover card and right-click render the same item list from one + // definition, so both are asserted against the same expectations. + const MENU_ITEMS = [ + "Pin", + "Rename", + "Add to Command Center", + "File to…", + "Archive", + ]; - render( + function renderWithMenu(overrides: { + onRename?: () => void; + onAddToCommandCenter?: () => void; + }) { + return render( {})} + onAddToCommandCenter={overrides.onAddToCommandCenter} /> , ); + } + + /** Hovers the row and waits for its preview card, which opens on a delay. */ + async function openCard() { + await userEvent.hover(screen.getByText("Investigate signup drop-off")); + return screen.findByRole("button", { name: "Pin" }, { timeout: 2000 }); + } + + it("puts the row's actions in the hover card", async () => { + renderWithMenu({}); + + await openCard(); + + for (const label of MENU_ITEMS) { + expect(screen.getByRole("button", { name: label })).not.toBeNull(); + } + }); + + it("opens the same menu on right-click", () => { + renderWithMenu({}); fireEvent.contextMenu(screen.getByText("Investigate signup drop-off")); - expect(onContextMenu).toHaveBeenCalledOnce(); + for (const label of MENU_ITEMS) { + expect(screen.getByRole("menuitem", { name: label })).not.toBeNull(); + } + }); + + it("disables Add to Command Center when there is nowhere to put the task", async () => { + renderWithMenu({ onAddToCommandCenter: undefined }); + + await openCard(); + + // Quill keeps a disabled button focusable, so the state is aria-disabled + // rather than the native attribute. + expect( + screen.getByRole("button", { name: "Add to Command Center" }), + ).toHaveAttribute("aria-disabled", "true"); + }); + + it("renames from the hover card", async () => { + const onRename = vi.fn(); + renderWithMenu({ onRename }); + + await openCard(); + await userEvent.click(screen.getByRole("button", { name: "Rename" })); + + expect(onRename).toHaveBeenCalledOnce(); + }); + + it("flashes a red dot while a canvas waits out its delete-undo window", () => { + const canvas = item({ + key: "canvas:c1", + kind: "canvas", + id: "c1", + title: "Web analytics overview", + }); + usePendingCanvasDeleteStore.getState().markPending("c1"); + + renderRow(canvas); + + expect(screen.getByRole("img", { name: "Deleting…" })).not.toBeNull(); + expect(screen.queryByRole("img", { name: "All caught up" })).toBeNull(); + }); + + it("gives a canvas the actions it has: pin and delete, not archive or filing", async () => { + const canvas = item({ + key: "canvas:c1", + kind: "canvas", + id: "c1", + title: "Web analytics overview", + }); + render( + + + , + ); + + await userEvent.hover(screen.getByText("Web analytics overview")); + + expect( + await screen.findByRole("button", { name: "Pin" }, { timeout: 2000 }), + ).not.toBeNull(); + expect(screen.getByRole("button", { name: "Delete…" })).not.toBeNull(); + // A canvas can't be archived, filed to a space, or given a command-centre + // cell, so those items aren't drawn at all rather than drawn dead. + for (const absent of ["Archive", "File to…", "Add to Command Center"]) { + expect(screen.queryByRole("button", { name: absent })).toBeNull(); + } + }); + + it("confirms before deleting a canvas — it goes for the whole space", async () => { + const remove = vi.fn(); + const canvas = item({ + key: "canvas:c1", + kind: "canvas", + id: "c1", + title: "Web analytics overview", + }); + render( + + + , + ); + + await userEvent.hover(screen.getByText("Web analytics overview")); + await userEvent.click( + await screen.findByRole("button", { name: "Delete…" }, { timeout: 2000 }), + ); + + // The menu item only opens the confirm; nothing is deleted until it is. + expect(remove).not.toHaveBeenCalled(); + await userEvent.click( + await screen.findByRole("button", { name: /^Delete$/ }), + ); + + expect(remove).toHaveBeenCalledWith(canvas); }); }); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx index ae5bc43ebb3e..195e40f385b2 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelItemRow.tsx @@ -1,21 +1,59 @@ import { PreviewCard } from "@base-ui/react/preview-card"; -import { Archive, FileTextIcon, PushPin } from "@phosphor-icons/react"; +import { ChatCircleIcon } from "@phosphor-icons/react"; import type { ChannelItemModel } from "@posthog/core/canvas/channelItems"; import { - isRunStatusActive, runStatusLabel, runStatusVariant, } from "@posthog/core/canvas/runStatus"; -import { Avatar, AvatarFallback, Badge } from "@posthog/quill"; +import { + AlertDialog, + AlertDialogClose, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + Avatar, + AvatarFallback, + AvatarGroup, + Badge, + Button, + Card, + Item, + ItemContent, + ItemDescription, + ItemGroup, + ItemMedia, + ItemSeparator, + ItemTitle, + Tooltip, + TooltipContent, + TooltipTrigger, +} from "@posthog/quill"; import { formatRelativeTimeShort } from "@posthog/shared"; import { UserAvatar } from "@posthog/ui/features/auth/UserAvatar"; import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; +import { + TaskRowContextMenu, + TaskRowMenuList, + type TaskRowMenuProps, +} from "@posthog/ui/features/canvas/components/TaskRowMenu"; +import { useChannelTaskStatus } from "@posthog/ui/features/canvas/hooks/useChannelTaskStatus"; +import { useIsCanvasPendingDelete } from "@posthog/ui/features/canvas/stores/pendingCanvasDeleteStore"; import { userDisplayName } from "@posthog/ui/features/canvas/utils/userDisplay"; import { InlineEditInput } from "@posthog/ui/features/sidebar/components/items/TaskItem"; +import { + PinnedBadge, + TaskBadgeStack, + TaskStatusDot, + TaskStatusTooltips, +} from "@posthog/ui/features/sidebar/components/items/TaskStatusDot"; +import { + type TaskDot, + taskDot, +} from "@posthog/ui/features/sidebar/components/items/taskStatusVocabulary"; import { SidebarItem } from "@posthog/ui/features/sidebar/components/SidebarItem"; -import { NestedButton } from "@posthog/ui/primitives/NestedButton"; -import { Tooltip } from "@posthog/ui/primitives/Tooltip"; -import type { ReactNode } from "react"; +import { type ReactNode, useCallback, useState } from "react"; /** * What a row can do. One object per channel rather than closures per item, so @@ -25,75 +63,181 @@ export interface ChannelItemActions { open: (item: ChannelItemModel) => void; togglePin: (item: ChannelItemModel) => void; archive: (item: ChannelItemModel) => void; + /** Canvases only — a task is archived, not deleted. */ + remove: (item: ChannelItemModel) => void; } // The channel sidebar's own chrome. Deliberately not shared with the Code // sidebar's TaskItem: that one is still on the absolute gray scale, while these // rows use the theme's fill/foreground tokens. -const HOVER_ACTION_CLASS = - "flex h-5 w-5 cursor-pointer items-center justify-center rounded text-muted-foreground transition-colors hover:bg-fill-hover hover:text-foreground"; -const HOVER_TOOLBAR_CLASS = - "hidden shrink-0 items-center gap-0.5 group-hover:flex"; -const TIMESTAMP_CLASS = - "shrink-0 text-[11px] text-muted-foreground group-hover:hidden"; +const TIMESTAMP_CLASS = "shrink-0 text-[11px] text-muted-foreground"; +// The badges own the trailing slot outright now that the actions have moved to +// the hover card — a row's identity is what you scan a task list for. The gap is +// between stacks (a pin, then the status badges), not within one. +const TRAILING_CLASS = "flex shrink-0 items-center gap-1"; -function itemIcon(item: ChannelItemModel): ReactNode { - return item.kind === "canvas" ? ( - // Matches the schema's own default for boards saved before templating. - iconForTemplate(item.templateId ?? "freeform", { - size: 15, - className: "text-violet-9", - }) - ) : ( - - ); +/** + * What the card leads with. A canvas gets its template glyph in canvas violet; a + * task gets the chat glyph the sidebar uses for a task with nothing going on — + * before this, a task was shown wearing a canvas's icon. + */ +function previewGlyph(item: ChannelItemModel): ReactNode { + if (item.kind !== "canvas") { + return ; + } + // Matches the schema's own default for boards saved before templating. + return iconForTemplate(item.templateId ?? "freeform", { + size: 15, + className: "text-violet-9", + }); } /** - * Marks a row whose run is still going. The glyph is kept and shimmered rather - * than swapped for a spinner, so the list stays scannable by kind while it - * moves — you can still tell a running task from a running canvas. + * A canvas waiting out its delete-undo window. Red and flashing because it is + * the one row state that is about to stop existing — everything else in this + * vocabulary is something you can come back to. */ -function RunningIcon({ children }: { children: ReactNode }) { - return ( - - {children} - - ); -} +const DELETING_DOT: TaskDot = { + tone: "red", + style: "solid", + pulse: true, + label: "Deleting…", +}; function authorLabel(item: ChannelItemModel): string | null { if (item.authorUser) return userDisplayName(item.authorUser); return item.authorName; } +/** + * One badge in a row's trailing stack, named on hover like the ones + * `TaskBadgeStack` draws — the row's tooltip provider is already up, so this + * shares its open delay. + */ +function RowBadge({ label, children }: { label: string; children: ReactNode }) { + return ( + + {/* `cursor-default`: a badge names a fact about the row, it isn't a + control — see the same note in TaskBadgeStack. */} + + + {children} + + + } + /> + + {label} + + + ); +} + +/** + * A canvas's trailing stack: the pin, then its template glyph. Same stack as a + * task's badges — a pinned canvas reads the way a pinned task does. + */ +function CanvasBadgeStack({ + item, + pinned, +}: { + item: ChannelItemModel; + pinned?: boolean; +}) { + return ( + + {pinned ? : null} + + {/* Violet is the canvas colour everywhere else it appears — the + artifacts list, the thread panel, the pinned menu — so the badge says + "canvas" the same way they do. */} + {iconForTemplate(item.templateId ?? "freeform", { + size: 9, + className: "text-violet-9", + })} + + + ); +} + export function ChannelItemRow({ item, + channelId, isActive, actions, isEditing = false, - onContextMenu, + onRename, + onAddToCommandCenter, onEditSubmit, onEditCancel, }: { item: ChannelItemModel; + /** The space this row is listed under, ticked in the menu's "File to…". */ + channelId?: string; isActive: boolean; actions: ChannelItemActions; isEditing?: boolean; - onContextMenu?: (event: React.MouseEvent) => void; + /** Puts the row into inline-rename mode. Absent for canvases. */ + onRename?: () => void; + /** Absent when the command centre has no free cell, which disables the item. */ + onAddToCommandCenter?: () => void; onEditSubmit?: (newTitle: string) => void; onEditCancel?: () => void; }) { - const icon = itemIcon(item); + const status = useChannelTaskStatus(item); + const [cardOpen, setCardOpen] = useState(false); + const [submenuOpen, setSubmenuOpen] = useState(false); + const [confirmDeleteOpen, setConfirmDeleteOpen] = useState(false); + // A canvas inside its undo window stays in the list rather than vanishing and + // reappearing on Undo, so the row has to say what's happening to it. + const pendingDelete = useIsCanvasPendingDelete(item.id); + const deleting = item.kind === "canvas" && pendingDelete; + // Stable: `TaskRowMenuList` builds its item components from these, so a new + // identity each render would remount every button in the card. + const closeCard = useCallback(() => setCardOpen(false), []); const statusLabel = runStatusLabel(item.rawStatus); const author = authorLabel(item); - // Only the row shimmers. The preview card spells the status out in a badge, - // so animating its copy of the icon would say the same thing twice. - const rowIcon = isRunStatusActive(item.rawStatus) ? ( - {icon} - ) : ( - icon + // The row's leading mark is always the task-list state vocabulary. Canvases + // have no live run, so they use the quiet dot and move their glyph to the + // right-side identity stack — except while one is being deleted, which is the + // one thing a canvas row has to shout. + const rowIcon = ( + ); + const previewIcon = previewGlyph(item); + // A canvas gets the same menu with the items it actually has: pin, and delete + // instead of archive. Filing and command-centre cells are task-shaped, and the + // menu drops them rather than showing them dead. + const menu: TaskRowMenuProps = + item.kind === "canvas" + ? { + kind: "canvas", + id: item.id, + title: item.title, + isPinned: item.pinned, + onTogglePin: () => actions.togglePin(item), + // Confirm first, like the canvas menus in the artifacts grid and the + // canvas header: the canvas and its history go for everyone. + onDelete: () => setConfirmDeleteOpen(true), + } + : { + kind: "task", + id: item.id, + title: item.title, + isPinned: item.pinned, + channelId, + onAddToCommandCenter, + onRename, + onTogglePin: () => actions.togglePin(item), + onArchive: () => actions.archive(item), + }; if (isEditing) { return ( @@ -108,8 +252,13 @@ export function ChannelItemRow({ ); } - return ( - + // One tooltip provider per task row, shared by its dot and badges so moving + // between them doesn't re-wait the open delay. Canvas rows have neither. + const row = ( + // Controlled so the card survives its own submenu: "File to…" opens in a + // portal outside the card, and the pointer moving there reads as leaving the + // card, which would take the menu down with it. + {item.title}} isActive={isActive} onClick={() => actions.open(item)} - onContextMenu={onContextMenu} endContent={ - <> - - {formatRelativeTimeShort(item.ts)} - - - - actions.togglePin(item)} - > - - - - {/* Canvases can't be archived. */} - {item.kind === "task" && ( - - actions.archive(item)} + + {/* Badges take the timestamp's slot on a task row: the row's + identity (pin, source, cloud, PR) is what you scan a task + list for, and the relative age is still in the preview + card. The pin joins whichever stack the row has, rather + than standing beside it as a badge of its own. */} + {status ? ( + + ) : item.kind === "canvas" ? ( + + ) : ( + <> + {item.pinned && ( + - - - - )} - - + + + )} + + {formatRelativeTimeShort(item.ts)} + + + )} + } />
@@ -167,52 +312,123 @@ export function ChannelItemRow({ sideOffset={10} className="z-50" > - -
- - {icon} - -
-

- {item.title} -

-

- {item.kind === "canvas" ? "Canvas" : "Task"} · updated{" "} - {formatRelativeTimeShort(item.ts)} -

-
-
- {statusLabel && ( -
- - {statusLabel} - -
- )} - {author && ( -
- {item.authorUser ? ( - - ) : ( - - - {author.charAt(0).toUpperCase()} - - - )} -
-

- {author} -

-

- Created by -

+ {/* The card is quill's `Card` and `Item` parts throughout — the popup + itself carries no surface styling, so this window's hover card + matches every other card in the app rather than a hand-tuned + shadow of its own. The card's own padding is off (`gap-0 py-0`): + each section pays for its own inset, which is what lets the rules + run edge to edge and the action rows highlight full width. */} + + } + > + + + + {previewIcon} + + + {item.title} + + {item.kind === "canvas" ? "Canvas" : "Task"} · updated{" "} + {formatRelativeTimeShort(item.ts)} + + + + {statusLabel && ( +
+ + {statusLabel} +
+ )} + {author && ( + <> + {/* Every section of the card gets the rule above it, canvases + included — the author is a different fact from the thing's + identity whether or not there are actions under it. */} + + + + {item.authorUser ? ( + + ) : ( + + + {author.charAt(0).toUpperCase()} + + + )} + + + {author} + Created by + + + + )} + {/* The row's actions live here now: a row at rest shows its + status, and the card is already the surface you're pointing at + when you want to do something to it. */} + +
+
- )} + ); + + const tipped = {row}; + // Right-click opens the same actions the hover card lists, from the same + // definition, so the two can't drift. + return ( + <> + {tipped} + {/* The same confirm the artifacts grid and the canvas header show: a + canvas goes for everyone in the space, so it isn't a one-click action + however small the row is. The undo window still follows. */} + + + + Delete canvas + + Delete {item.title}? Its code + and version history go for everyone in the space. You get a few + seconds to undo, then it's permanent. + + + + + Cancel + + } + /> + + + + + + ); } diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.test.tsx index fe2da703cb26..c0a07f916289 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.test.tsx @@ -74,9 +74,48 @@ describe("ChannelNav", () => { const activity = screen.getByLabelText("Activity"); expect(activity).toBeEnabled(); + expect(activity).not.toHaveAttribute("aria-haspopup"); await user.hover(activity); await new Promise((resolve) => setTimeout(resolve, 400)); expect(screen.queryByText("Recent activity card")).not.toBeInTheDocument(); }); + + it("leaves no popover state on the bell after it navigates to Activity", async () => { + const user = userEvent.setup(); + const { rerender } = render(); + const bell = () => screen.getByLabelText("Activity"); + + await user.hover(bell()); + await screen.findByText("Recent activity card", {}, { timeout: 1_000 }); + await user.click(bell()); + mocks.view = { type: "activity" }; + rerender(); + + expect(screen.queryByText("Recent activity card")).not.toBeInTheDocument(); + expect(bell()).not.toHaveAttribute("data-popup-open"); + expect(bell()).not.toHaveAttribute("data-pressed"); + }); + + it("neither resurfaces nor wedges the hover card once the bell has navigated", async () => { + const user = userEvent.setup(); + const { rerender } = render(); + const bell = () => screen.getByLabelText("Activity"); + + await user.hover(bell()); + await user.click(bell()); + mocks.view = { type: "activity" }; + rerender(); + await user.unhover(bell()); + + mocks.view = { type: "task-detail" }; + rerender(); + await new Promise((resolve) => setTimeout(resolve, 400)); + expect(screen.queryByText("Recent activity card")).not.toBeInTheDocument(); + + await user.hover(bell()); + expect( + await screen.findByText("Recent activity card", {}, { timeout: 1_000 }), + ).toBeInTheDocument(); + }); }); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.tsx index fe76afeb198e..2c2f6377066e 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelNav.tsx @@ -39,7 +39,12 @@ import { } from "@posthog/ui/router/navigationBridge"; import { useAppView } from "@posthog/ui/router/useAppView"; import { track } from "@posthog/ui/shell/analytics"; -import { type ComponentPropsWithRef, type ReactNode, useState } from "react"; +import { + type ComponentPropsWithRef, + type ReactElement, + type ReactNode, + useState, +} from "react"; import { ActivityHoverCard } from "./ActivityHoverCard"; const INBOX_REFETCH_INTERVAL_MS = 60_000; @@ -133,10 +138,51 @@ function NavButton({ ); } +function ActivityHoverPopover({ trigger }: { trigger: ReactElement }) { + const [open, setOpen] = useState(false); + + return ( + + event.preventBaseUIHandler()} + render={trigger} + /> + {open && ( + setOpen(false)} /> + )} + + ); +} + +function ActivityNavItem({ + isActive, + unreadCount, + onNavigate, +}: { + isActive: boolean; + unreadCount: number; + onNavigate: () => void; +}) { + const bell = ( + } + label="Activity" + isActive={isActive} + onClick={onNavigate} + badge={} + /> + ); + + if (isActive) return bell; + return ; +} + export function ChannelNav() { const view = useAppView(); const loopsEnabled = useFeatureFlag(LOOPS_FLAG, import.meta.env.DEV); - const [activityOpen, setActivityOpen] = useState(false); const { counts } = useInboxAllReports({ ignoreFilters: true, @@ -165,7 +211,7 @@ export function ChannelNav() { // cannot do that — the skip window is provider state, and isolated // providers never share it. -
+
@@ -178,44 +224,11 @@ export function ChannelNav() { } /> - setActivityOpen(!isActivity && open)} - > - - } - label="Activity" - isActive={isActivity} - onClick={() => { - setActivityOpen(false); - withTrack("activity", navigateToActivity)(); - }} - badge={ - - } - /> - } - /> - {!isActivity && activityOpen && ( - setActivityOpen(false)} - /> - )} - + ({ items: [] as ChannelItemModel[], @@ -36,15 +36,11 @@ vi.mock("@posthog/ui/features/canvas/components/ChannelsFab", () => ({ ChannelsFab: () => null, })); -// The row context menu's hooks reach for a QueryClient and the DI container, -// neither of which a unit test has. Stubbed at the module boundary, as -// WebsiteLayout.test.tsx does for the same reason. -vi.mock("@posthog/ui/features/tasks/useTaskContextMenu", () => ({ - useTaskContextMenu: () => ({ - showContextMenu: vi.fn(), - editingTaskId: null, - setEditingTaskId: vi.fn(), - }), +// The row menu's spaces list reaches for a QueryClient the unit test has no +// stack for. Stubbed at the module boundary, as WebsiteLayout.test.tsx does for +// the same reason. +vi.mock("@posthog/ui/features/canvas/hooks/useChannels", () => ({ + useChannels: () => ({ channels: [] }), })); vi.mock("@posthog/ui/features/tasks/useTaskMutations", () => ({ useRenameTask: () => ({ renameTask: vi.fn() }), @@ -52,6 +48,10 @@ vi.mock("@posthog/ui/features/tasks/useTaskMutations", () => ({ vi.mock("@posthog/ui/features/tasks/useTasks", () => ({ useTasks: () => ({ data: [] }), })); +// A row's status dot reaches for live session state and a per-task PR query. +vi.mock("@posthog/ui/features/canvas/hooks/useChannelTaskStatus", () => ({ + useChannelTaskStatus: () => null, +})); import { ChannelSidebar } from "./ChannelSidebar"; @@ -69,6 +69,7 @@ function item(overrides: Partial = {}): ChannelItemModel { // Not the viewer, so filtering to "Me" leaves nothing. authorUuid: "someone-else-uuid", templateId: null, + task: null, ...overrides, }; } @@ -98,18 +99,18 @@ describe("ChannelSidebar", () => { what: "nothing has arrived yet", state: { items: [], isLoading: true }, shown: [] as string[], - hidden: ["Recent", "No matches", "Nothing here yet"], + hidden: ["Sessions", "No matches", "Nothing here yet"], }, { what: "the space is settled and genuinely empty", state: { items: [], isLoading: false }, shown: ["Nothing here yet"], - hidden: ["Recent", "No matches"], + hidden: ["Sessions", "No matches"], }, { what: "the space is settled with items", state: { items: [item()], isLoading: false }, - shown: ["Recent", "Investigate signup drop-off"], + shown: ["Sessions", "Investigate signup drop-off"], hidden: ["No matches", "Nothing here yet"], }, ])("shows one state when $what", ({ state, shown, hidden }) => { @@ -162,3 +163,100 @@ describe("ChannelSidebar", () => { expect(screen.queryByText("Nothing here yet")).not.toBeInTheDocument(); }); }); + +describe("ChannelSidebar recents list", () => { + const NOW = new Date(2026, 6, 29, 12); + + beforeEach(() => { + mocks.isLoading = false; + mocks.channelMissing = false; + vi.useFakeTimers({ shouldAdvanceTime: true }); + vi.setSystemTime(NOW); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("lists recents newest first without day separators", () => { + mocks.items = [ + item({ + key: "task:a", + id: "a", + title: "Today's work", + ts: new Date(2026, 6, 29, 9).getTime(), + }), + item({ + key: "task:b", + id: "b", + title: "Also today", + ts: new Date(2026, 6, 29, 8).getTime(), + }), + item({ + key: "task:c", + id: "c", + title: "Yesterday's work", + ts: new Date(2026, 6, 28, 17).getTime(), + }), + item({ + key: "task:d", + id: "d", + title: "Older work", + ts: new Date(2026, 6, 20, 17).getTime(), + }), + ]; + + renderSidebar(); + + expect(screen.getByText("Today's work")).not.toBeNull(); + expect(screen.getByText("Also today")).not.toBeNull(); + expect(screen.getByText("Yesterday's work")).not.toBeNull(); + expect(screen.getByText("Older work")).not.toBeNull(); + expect(screen.queryByText("Today")).toBeNull(); + expect(screen.queryByText("Yesterday")).toBeNull(); + expect(screen.queryByText("Monday, July 20th")).toBeNull(); + }); + + it("keeps items from the same day as plain rows", () => { + mocks.items = [ + item({ key: "task:a", id: "a", ts: new Date(2026, 6, 29, 9).getTime() }), + item({ key: "task:b", id: "b", ts: new Date(2026, 6, 29, 8).getTime() }), + item({ key: "task:c", id: "c", ts: new Date(2026, 6, 29, 1).getTime() }), + ]; + + renderSidebar(); + + expect(screen.getAllByText("Investigate signup drop-off")).toHaveLength(3); + expect(screen.queryByText("Today")).toBeNull(); + }); + + it("lists pins in the one session list, ahead of newer items", () => { + mocks.items = [ + item({ + key: "task:newer", + id: "newer", + title: "Filed this morning", + ts: new Date(2026, 6, 30, 9).getTime(), + }), + item({ + key: "task:pinned", + id: "pinned", + title: "Kept at hand", + pinned: true, + ts: new Date(2026, 6, 20, 9).getTime(), + }), + ]; + + renderSidebar(); + + // No section of its own — a pin is a mark on a session, and the row's badge + // is what says so. + expect(screen.queryByText("Pinned")).toBeNull(); + const titles = screen + .getAllByText(/Kept at hand|Filed this morning/) + .map((el) => el.textContent); + // Older, but pinned: it sorts above the newer row rather than risking the + // recents cap. + expect(titles).toEqual(["Kept at hand", "Filed this morning"]); + }); +}); diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelSidebar.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelSidebar.tsx index c069e1ac7da0..a2e8bb520bac 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelSidebar.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelSidebar.tsx @@ -8,6 +8,7 @@ import type { CreatedByFilter } from "@posthog/core/canvas/channelItems"; import { filterChannelItems } from "@posthog/core/canvas/channelItems"; import { RUN_STATUS_FILTER_OPTIONS } from "@posthog/core/canvas/runStatus"; import { + Button, cn, DropdownMenu, DropdownMenuContent, @@ -32,14 +33,14 @@ import { ChannelItemRow } from "@posthog/ui/features/canvas/components/ChannelIt import { ChannelsFab } from "@posthog/ui/features/canvas/components/ChannelsFab"; import { type ChannelPageKey, - channelPageIcon, channelPageLabel, } from "@posthog/ui/features/canvas/components/channelPages"; import { useChannelItems } from "@posthog/ui/features/canvas/hooks/useChannelItems"; +import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; +import { PERSONAL_CHANNEL_NAME } from "@posthog/ui/features/canvas/hooks/useTaskChannels"; import { useCommandCenterStore } from "@posthog/ui/features/command-center/commandCenterStore"; import { useFeatureFlag } from "@posthog/ui/features/feature-flags/useFeatureFlag"; import { SidebarItem } from "@posthog/ui/features/sidebar/components/SidebarItem"; -import { useTaskContextMenu } from "@posthog/ui/features/tasks/useTaskContextMenu"; import { useRenameTask } from "@posthog/ui/features/tasks/useTaskMutations"; import { useTasks } from "@posthog/ui/features/tasks/useTasks"; import { navigateToCommandCenter } from "@posthog/ui/router/navigationBridge"; @@ -54,11 +55,11 @@ const CREATED_BY_OPTIONS: readonly { value: CreatedByFilter; label: string }[] = { value: "others", label: "Other people" }, ] as const; -const HEADER_ICON_BUTTON_CLASS = - "flex size-5 shrink-0 items-center justify-center rounded text-muted-foreground transition-colors hover:bg-fill-hover hover:text-foreground"; - +// The header's icon buttons are quill's ghost button at the 20px scale; only the +// sticky state is ours, because quill styles the transient open state (hover, +// popup) but has no notion of "search is showing" or "a filter is applied". const cnHeaderButton = (active: boolean) => - cn(HEADER_ICON_BUTTON_CLASS, active && "bg-fill-selected text-foreground"); + cn("text-muted-foreground", active && "bg-fill-selected text-foreground"); const RECENTS_CAP = 30; const log = logger.scope("channel-sidebar"); @@ -70,6 +71,7 @@ function RecentSectionHeader({ onQueryChange, createdByFilter, onCreatedByChange, + showCreatedBy, statusFilter, onStatusChange, filtersActive, @@ -80,35 +82,39 @@ function RecentSectionHeader({ onQueryChange: (value: string) => void; createdByFilter: CreatedByFilter; onCreatedByChange: (value: CreatedByFilter) => void; + /** False in #me, where every session is yours and the filter says nothing. */ + showCreatedBy: boolean; statusFilter: TaskRunStatus | null; onStatusChange: (value: TaskRunStatus | null) => void; filtersActive: boolean; }) { return ( <> -
+
- Recent + Sessions
- + - + } /> - Created by - - onCreatedByChange(value as CreatedByFilter) - } - > - {CREATED_BY_OPTIONS.map((option) => ( - - {option.label} - - ))} - - + {/* #me holds only your own sessions, so "created by" can only ever + answer "you" — the whole group is dropped rather than shown with + two options that empty the list. */} + {showCreatedBy && ( + <> + Created by + + onCreatedByChange(value as CreatedByFilter) + } + > + {CREATED_BY_OPTIONS.map((option) => ( + + {option.label} + + ))} + + + + )} Status onQueryChange(event.target.value)} placeholder="Search…" - aria-label="Search recent items" + aria-label="Search sessions" className="h-6 text-[12px]" />
@@ -175,7 +191,7 @@ const SKELETON_ROW_WIDTHS = [60, 80, 40, 75, 50, 66] as const; function ChannelItemsSkeleton() { return (
- {/* Stands in for the "Recent" MenuLabel, so it carries that label's scale. */} + {/* Stands in for the "Sessions" MenuLabel, so it carries that label's scale. */} (null); const { renameTask } = useRenameTask(); const commandCenterCells = useCommandCenterStore((state) => state.cells); const assignTaskToCommandCenter = useCommandCenterStore( @@ -255,7 +273,17 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { const [createdByFilter, setCreatedByFilter] = useState("anyone"); const [statusFilter, setStatusFilter] = useState(null); - const filtersActive = createdByFilter !== "anyone" || statusFilter !== null; + // Every session in #me is yours, so the author filter has nothing to sort by. + // The state survives a space switch, so the value is neutralised here as well + // as hidden — otherwise "Other people" carried in from a shared space would + // empty this list with no visible control to undo it. + const { channels } = useChannels(); + const isPersonalChannel = + channels.find((c) => c.id === channelId)?.name === PERSONAL_CHANNEL_NAME; + const createdBy: CreatedByFilter = isPersonalChannel + ? "anyone" + : createdByFilter; + const filtersActive = createdBy !== "anyone" || statusFilter !== null; const base = `/website/${channelId}`; // Activeness is a key comparison rather than a flag baked into each item, so @@ -267,15 +295,22 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { return task ? `task:${task[1]}` : null; }, [pathname]); - const pinnedItems = useMemo(() => items.filter((i) => i.pinned), [items]); - const recentItems = useMemo( - () => - filterChannelItems( - items.filter((i) => !i.pinned), - { query, createdBy: createdByFilter, status: statusFilter, me }, - ).slice(0, RECENTS_CAP), - [items, query, createdByFilter, statusFilter, me], - ); + // One list, pins included — a pin is a mark on a session, not a different kind + // of thing, and the row's own badge says so. They sort to the top because a pin + // is a request not to lose the thing: below the recency order it would fall off + // the end of the cap. + const recentItems = useMemo(() => { + const matching = filterChannelItems(items, { + query, + createdBy, + status: statusFilter, + me, + }); + return [ + ...matching.filter((i) => i.pinned), + ...matching.filter((i) => !i.pinned), + ].slice(0, RECENTS_CAP); + }, [items, query, createdBy, statusFilter, me]); const narrowed = filtersActive || searchOpen; const listState = listStateOf({ @@ -284,41 +319,40 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { itemCount: items.length, narrowed, }); - // The list's two sections, which only exist once there are items. With - // everything pinned there's nothing left to list — but keep the header while - // it's narrowed, so you can undo whatever emptied it. - const showPinned = listState === "ready" && pinnedItems.length > 0; - const showRecent = - listState === "ready" && (items.some((i) => !i.pinned) || narrowed); + // The one section, which only exists once there are items — but its header + // stays while the list is narrowed, so you can undo whatever emptied it. + const showRecent = listState === "ready"; + + // The first free command-centre cell, or nothing if every cell is taken by a + // task that still exists. + const commandCenterAssigner = (taskId: string) => { + const cellIndex = commandCenterCells.findIndex( + (cellTaskId) => cellTaskId == null || !allTaskIds.has(cellTaskId), + ); + if (cellIndex === -1) return undefined; + return () => { + assignTaskToCommandCenter(cellIndex, taskId); + navigateToCommandCenter(); + }; + }; const taskRow = (item: (typeof items)[number]) => ( - void showContextMenu(item, event, { - isPinned: item.pinned, - isInCommandCenter: commandCenterCells.includes(item.id), - hasEmptyCommandCenterCell: commandCenterCells.some( - (taskId) => taskId == null || !allTaskIds.has(taskId), - ), - showArchivePrior: false, - onTogglePin: () => actions.togglePin(item), - onArchive: () => actions.archive(item), - onAddToCommandCenter: () => { - const cellIndex = commandCenterCells.findIndex( - (taskId) => taskId == null || !allTaskIds.has(taskId), - ); - if (cellIndex === -1) return; - assignTaskToCommandCenter(cellIndex, item.id); - navigateToCommandCenter(); - }, - }) + onRename={ + item.kind === "task" ? () => setEditingTaskId(item.id) : undefined + } + // Undefined disables the menu item: a full command centre has nowhere to + // put the task, and an action that silently does nothing is worse than a + // greyed-out one. + onAddToCommandCenter={ + item.kind === "task" && !commandCenterCells.includes(item.id) + ? commandCenterAssigner(item.id) : undefined } onEditSubmit={ @@ -341,8 +375,10 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { /> ); - // Label and icon come from the shared space-page table, so a sidebar row and - // the header breadcrumb for the same page can never disagree. + // Label comes from the shared space-page table, so a sidebar row and the + // header breadcrumb for the same page can never disagree. No icon: this is a + // four-row list of words, and glyphs here only compete with the status dots + // in the sessions list below for the eye's attention. const sectionRow = ( page: ChannelPageKey, to: string, @@ -350,7 +386,6 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { ) => ( +
+ {/* Starting a session is what you came here to do, so it leads the + pane's list of places rather than hiding behind one of them. */} + + void navigate({ + to: "/website/$channelId/new", + params: { channelId }, + }) + } + /> {sectionRow( "home", base, @@ -420,15 +468,6 @@ export function ChannelSidebar({ channelId }: { channelId: string }) { )} - {showPinned && ( - <> - Pinned -
- {pinnedItems.map(taskRow)} -
- - )} - {showRecent && ( <> { useSidebarStore.setState({ collapsedSections: new Set() }); }); + it("opens a space in the sidebar without navigating the main window", async () => { + const user = userEvent.setup(); + renderList(); + + await user.click(screen.getByText("engineering")); + + expect(useCurrentChannelStore.getState().currentChannelId).toBe(ENG.id); + expect(mocks.navigate).not.toHaveBeenCalled(); + }); + it("pins #me above the channels, with its ⌘1 shortcut", () => { renderList(); const me = screen.getByText("me"); @@ -182,10 +193,8 @@ describe("ChannelsList", () => { await user.type(screen.getByLabelText("Search spaces"), "eng"); await user.keyboard("{Enter}"); - expect(mocks.navigate).toHaveBeenCalledWith({ - to: "/website/$channelId", - params: { channelId: ENG.id }, - }); + expect(useCurrentChannelStore.getState().currentChannelId).toBe(ENG.id); + expect(mocks.navigate).not.toHaveBeenCalled(); }); it("moves the highlight with the arrow keys", async () => { @@ -197,10 +206,8 @@ describe("ChannelsList", () => { await user.type(screen.getByLabelText("Search spaces"), "e"); await user.keyboard("{ArrowDown}{Enter}"); - expect(mocks.navigate).toHaveBeenCalledWith({ - to: "/website/$channelId", - params: { channelId: ENG.id }, - }); + expect(useCurrentChannelStore.getState().currentChannelId).toBe(ENG.id); + expect(mocks.navigate).not.toHaveBeenCalled(); }); // Base UI's clear button is a tabIndex=-1 decoration by default, which left @@ -247,10 +254,8 @@ describe("ChannelsList", () => { await user.click(screen.getByLabelText("Search spaces")); await user.keyboard("{ArrowDown}{Enter}"); - expect(mocks.navigate).toHaveBeenCalledWith({ - to: "/website/$channelId", - params: { channelId: ENG.id }, - }); + expect(useCurrentChannelStore.getState().currentChannelId).toBe(ENG.id); + expect(mocks.navigate).not.toHaveBeenCalled(); }); // Base UI resets the highlight when the pointer leaves a row, and @@ -266,10 +271,8 @@ describe("ChannelsList", () => { await user.unhover(row); await user.keyboard("{Enter}"); - expect(mocks.navigate).toHaveBeenCalledWith({ - to: "/website/$channelId", - params: { channelId: ENG.id }, - }); + expect(useCurrentChannelStore.getState().currentChannelId).toBe(ENG.id); + expect(mocks.navigate).not.toHaveBeenCalled(); }); // A kept-mounted collapsed row would still be an option, so ↓ would walk @@ -323,10 +326,8 @@ describe("ChannelsList", () => { // it would have been the row after it. await user.keyboard("{ArrowDown}{Enter}"); - expect(mocks.navigate).toHaveBeenCalledWith({ - to: "/website/$channelId", - params: { channelId: ENG.id }, - }); + expect(useCurrentChannelStore.getState().currentChannelId).toBe(ENG.id); + expect(mocks.navigate).not.toHaveBeenCalled(); }); it("selects a stale query so the next keystroke replaces it", async () => { diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx index 57a7191976a5..9be83a64a60c 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelsList.tsx @@ -143,7 +143,7 @@ function SpaceRowSurface({ - {/* A single, non-expandable row: the "# name" navigates straight to the - channel home. Right-clicking opens the same actions as the "..." menu. */} + {/* A single, non-expandable row: the "# name" opens the channel sidebar. + Right-clicking opens the same actions as the "..." menu. */} {channel.name} + {/* `!mr-0` undoes quill's `.quill-button kbd { margin-right: -4px }`, + which is meant to let a shortcut hang into a button's own + padding. Here the row's inner span is `truncate` (overflow + hidden) and `ml-auto` eats every pixel of slack, so the hang + had nowhere to go and the last 4px of the hint was cut off. */} {hotkeySlot != null && ( - + {formatHotkey(`mod+${hotkeySlot}`)} )} @@ -630,6 +635,7 @@ function useOpenPersonalChannel(): { openPersonalChannel: () => Promise; isCreating: boolean; } { + const spacesLayout = useChannelsLayout(); const navigate = useNavigate(); const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); const { channels } = useChannels(); @@ -651,18 +657,20 @@ function useOpenPersonalChannel(): { if (!channelId) return; showChannelPane(); setCurrentChannel(channelId); - void navigate({ to: "/website/$channelId", params: { channelId } }); + if (!spacesLayout) { + void navigate({ to: "/website/$channelId", params: { channelId } }); + } }; return { ensureFolderId, openPersonalChannel, isCreating }; } /** - * Navigating into a channel, shared by the tree rows and the search results. - * Slides before navigating: the route effect would get there too, but not until - * the navigation resolves. + * Opening a channel, shared by the tree rows and the search results. In the + * Spaces layout this scopes the sidebar without moving the main window. */ function useOpenChannel(): (channel: Channel) => void { + const spacesLayout = useChannelsLayout(); const navigate = useNavigate(); const setCurrentChannel = useCurrentChannelStore((s) => s.setCurrentChannel); @@ -674,10 +682,12 @@ function useOpenChannel(): (channel: Channel) => void { }); showChannelPane(); setCurrentChannel(channel.id); - void navigate({ - to: "/website/$channelId", - params: { channelId: channel.id }, - }); + if (!spacesLayout) { + void navigate({ + to: "/website/$channelId", + params: { channelId: channel.id }, + }); + } }; } @@ -757,7 +767,7 @@ function PersonalChannelRow({ hotkeySlot }: { hotkeySlot?: number }) { {PERSONAL_CHANNEL_NAME} {hotkeySlot != null && ( - + {formatHotkey(`mod+${hotkeySlot}`)} )} diff --git a/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx b/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx index d9f75f09479c..05f0d80851e0 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/ChannelsSidebar.tsx @@ -234,6 +234,13 @@ export function ChannelsSidebar() { {channelsLayout ? ( <> + {/* Which project you're in is the outermost thing about this window, + so under the layout it sits above the nav row rather than in the + footer. Its menu opens downward, which is the right direction + from the top of a sidebar. */} + + + @@ -270,9 +277,13 @@ export function ChannelsSidebar() { - - - + {/* The code layout keeps it in the footer: that sidebar's top is the nav + section and task header, and there's no nav row to sit above. */} + {!channelsLayout && ( + + + + )} ); diff --git a/products/desktop/packages/ui/src/features/canvas/components/TaskRowMenu.tsx b/products/desktop/packages/ui/src/features/canvas/components/TaskRowMenu.tsx new file mode 100644 index 000000000000..f3e3e4310e41 --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/components/TaskRowMenu.tsx @@ -0,0 +1,226 @@ +import { CaretRightIcon } from "@phosphor-icons/react"; +import { + Button, + ContextMenu, + ContextMenuContent, + ContextMenuItem, + ContextMenuSub, + ContextMenuSubTrigger, + ContextMenuTrigger, + DropdownMenu, + DropdownMenuTrigger, +} from "@posthog/quill"; +import { PROJECT_BLUEBIRD_FLAG } from "@posthog/shared"; +import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; +import { useFileTaskToChannel } from "@posthog/ui/features/canvas/hooks/useFileTaskToChannel"; +import { useFeatureFlag } from "@posthog/ui/features/feature-flags/useFeatureFlag"; +import { + type MenuFlyoutItem, + MenuSubFlyout, + SearchableMenuFlyout, +} from "@posthog/ui/primitives/SearchableMenuFlyout"; +import { type ComponentType, type ReactNode, useMemo } from "react"; + +/** + * What a row's menu can do. The row owns the handlers because they're the same + * ones its list already has (pin, archive, delete, rename inline); only filing — + * which needs the channel list and a mutation — belongs to the menu. + * + * Canvases share this menu but not all of it: they can be pinned and deleted, + * and they can't be filed to a space or given a command-centre cell, both of + * which are task-shaped. `kind` is what decides, so a canvas gets a menu of the + * actions it has rather than a full one with half its items dead. + */ +export interface TaskRowMenuProps { + kind: "task" | "canvas"; + id: string; + title: string; + isPinned: boolean; + /** The channel this task is already filed to, ticked in "File to…". */ + channelId?: string; + /** Absent when the command centre is full, which disables the item. */ + onAddToCommandCenter?: () => void; + /** Absent where there's no inline rename to open — canvases, for now. */ + onRename?: () => void; + onTogglePin: () => void; + /** Tasks are archived; canvases are deleted (with an undo window). */ + onArchive?: () => void; + onDelete?: () => void; +} + +// The two menus differ only in which primitives draw them, so the item list is +// written once against this shape. Base UI builds context menus on the same Menu +// parts as dropdowns, so the props line up; typing them structurally keeps the +// shared content from having to know which surface it's on. +interface MenuParts { + Item: ComponentType<{ + children: ReactNode; + disabled?: boolean; + variant?: "default" | "destructive"; + onClick?: () => void; + }>; + Sub: ComponentType<{ children: ReactNode }>; + SubTrigger: ComponentType<{ children: ReactNode }>; +} + +const CONTEXT_PARTS: MenuParts = { + Item: ContextMenuItem, + Sub: ContextMenuSub, + SubTrigger: ContextMenuSubTrigger, +}; + +/** + * The row's actions, in the order the native menu used: the edits, then the + * places a task can be sent, then the destructive one last. + */ +function TaskRowMenuItems({ + parts, + menu, +}: { + parts: MenuParts; + menu: TaskRowMenuProps; +}) { + const { Item, Sub, SubTrigger } = parts; + // "File to…" is a Project Bluebird feature; gate the channel fetch behind the + // flag so neither the submenu nor its request reaches ungated users. + const bluebirdEnabled = useFeatureFlag( + PROJECT_BLUEBIRD_FLAG, + import.meta.env.DEV, + ); + const isTask = menu.kind === "task"; + const { channels } = useChannels({ enabled: bluebirdEnabled && isTask }); + const fileToChannel = useFileTaskToChannel(); + + const channelItems: MenuFlyoutItem[] = channels.map((channel) => ({ + id: channel.id, + label: channel.name, + current: channel.id === menu.channelId, + })); + + return ( + <> + {menu.isPinned ? "Unpin" : "Pin"} + {menu.onRename && Rename} + {isTask && ( + + Add to Command Center + + )} + {isTask && channelItems.length > 0 && ( + + File to… + + + fileToChannel(channelId, menu.id, menu.title) + } + /> + + + )} + {menu.onArchive && Archive} + {/* The ellipsis is the promise that a confirm follows — deleting a canvas + takes it away from everyone in the space. */} + {menu.onDelete && ( + + Delete… + + )} + + ); +} + +/** + * The same actions as a plain list, for a surface that is already open — the + * row's hover card. Rows are quill buttons rather than menu items because + * nothing here is a popup: there's no menu root to give `DropdownMenuItem` its + * keyboard handling, and a button is what quill offers for a click target in a + * card. + * + * `onAction` closes the surface once something has been chosen, and + * `onSubmenuOpenChange` reports the one thing that *is* a popup ("File to…"), so + * a hover surface can stay open while the pointer is inside it. + */ +export function TaskRowMenuList({ + menu, + onAction, + onSubmenuOpenChange, +}: { + menu: TaskRowMenuProps; + onAction: () => void; + onSubmenuOpenChange: (open: boolean) => void; +}) { + const parts: MenuParts = useMemo( + () => ({ + Item: ({ children, disabled, variant, onClick }) => ( + + ), + Sub: ({ children }) => ( + + {children} + + ), + // `openOnHover`, so the spaces flyout arrives the way a submenu does in + // the right-click menu — pointing at the row is the whole gesture, and + // this card is a hover surface to begin with. + SubTrigger: ({ children }) => ( + + {children} + + + } + /> + ), + }), + [onAction, onSubmenuOpenChange], + ); + + return ( +
+ +
+ ); +} + +/** The same menu on right-click, wrapping the row. */ +export function TaskRowContextMenu({ + menu, + children, +}: { + menu: TaskRowMenuProps; + children: ReactNode; +}) { + return ( + + }> + {children} + + + + + + ); +} diff --git a/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelHome.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelHome.test.tsx new file mode 100644 index 000000000000..a8ccd8d356ad --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelHome.test.tsx @@ -0,0 +1,116 @@ +import { Theme } from "@radix-ui/themes"; +import { act, render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +if (typeof globalThis.ResizeObserver === "undefined") { + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver; +} + +vi.mock("@posthog/ui/shell/rendererStorage", () => ({ + electronStorage: { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + }, +})); +vi.mock("@posthog/ui/features/canvas/hooks/useChannels", () => ({ + useChannels: () => ({ + channels: [{ id: "chan-1", name: "eng" }], + isLoading: false, + }), +})); +vi.mock("@posthog/ui/features/canvas/hooks/useChannelsLayout", () => ({ + useChannelsLayout: () => true, +})); +vi.mock("@posthog/ui/features/canvas/hooks/useTaskChannels", () => ({ + PERSONAL_CHANNEL_NAME: "me", + useBackendChannel: () => ({ + channel: { id: "backend-1", name: "eng" }, + isLoading: false, + }), +})); +vi.mock("@posthog/ui/features/canvas/hooks/useChannelFeed", () => ({ + useChannelFeed: () => ({ tasks: [], isLoading: false }), + channelFeedQueryKey: () => ["feed"], +})); +vi.mock("@posthog/ui/features/canvas/hooks/useChannelFeedMessages", () => ({ + useChannelFeedMessages: () => ({ messages: [], isLoading: false }), + channelCreationMessage: () => null, +})); +vi.mock("@posthog/ui/features/canvas/hooks/useFolderInstructions", () => ({ + useFolderInstructions: () => ({ data: undefined, isLoading: false }), +})); +vi.mock("@posthog/ui/features/canvas/hooks/useChannelTasks", () => ({ + useChannelTaskMutations: () => ({ fileTask: () => Promise.resolve() }), +})); +vi.mock("@posthog/ui/hooks/useSetHeaderContent", () => ({ + useSetHeaderContent: () => {}, +})); +vi.mock("@posthog/ui/shell/analytics", () => ({ track: vi.fn() })); +vi.mock("@tanstack/react-query", () => ({ + useQueryClient: () => ({ setQueryData: vi.fn(), invalidateQueries: vi.fn() }), +})); +vi.mock("@tanstack/react-router", () => ({ useNavigate: () => vi.fn() })); + +// ThreadSidebar is the task dock under test; the rest of the channel chrome +// (feed rows, composer, intro) plays no part in the feed/sidebar exclusion. +vi.mock("@posthog/ui/features/canvas/components/ChannelFeedView", () => ({ + ChannelFeedView: () =>
, +})); +vi.mock("@posthog/ui/features/canvas/components/ChannelHomeComposer", () => ({ + ChannelHomeComposer: () => null, +})); +vi.mock("@posthog/ui/features/canvas/components/ChannelIntro", () => ({ + ChannelIntro: () => null, +})); +vi.mock("@posthog/ui/features/canvas/components/CreateChannelModal", () => ({ + CreateChannelModal: () => null, +})); +vi.mock("@posthog/ui/features/canvas/components/ThreadSidebar", () => ({ + ThreadSidebar: () =>
, +})); + +import { useThreadPanelStore } from "@posthog/ui/features/canvas/stores/threadPanelStore"; +import { WebsiteChannelHome } from "./WebsiteChannelHome"; + +describe("WebsiteChannelHome", () => { + beforeEach(() => { + useThreadPanelStore.setState({ + openByChannel: {}, + collapsed: false, + width: 360, + }); + }); + + it("drops a stale open thread so the feed can't show a task sidebar", () => { + useThreadPanelStore.getState().openThread("chan-1", "task-1"); + render( + + + , + ); + + expect(screen.getByTestId("feed")).toBeTruthy(); + expect(screen.queryByTestId("task-sidebar")).toBeNull(); + expect(useThreadPanelStore.getState().openByChannel["chan-1"]).toBeNull(); + }); + + it("shows the task sidebar for a thread opened from this feed", () => { + render( + + + , + ); + + act(() => { + useThreadPanelStore.getState().openThread("chan-1", "task-1"); + }); + + expect(screen.getByTestId("task-sidebar")).toBeTruthy(); + expect(screen.queryByTestId("feed")).toBeTruthy(); + }); +}); diff --git a/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelHome.tsx b/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelHome.tsx index 3195ba4877d2..e34e11a7d255 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelHome.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelHome.tsx @@ -44,7 +44,7 @@ import { track } from "@posthog/ui/shell/analytics"; import { Heading, Text } from "@radix-ui/themes"; import { useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "@tanstack/react-router"; -import { useCallback, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; // A channel: a Slack-style multiplayer feed. Each member message kicks off a // task rendered as a card everyone in the channel sees; the composer stays @@ -135,6 +135,19 @@ export function WebsiteChannelHome({ channelId }: { channelId: string }) { const openThread = useThreadPanelStore((s) => s.openThread); const closeThread = useThreadPanelStore((s) => s.closeThread); + // The open thread outlives the thread view, so the feed showing itself is + // the only signal an inherited thread is gone. Suppress it in render (an + // effect alone would paint the sidebar for a frame first) and clear the + // store; threads opened from this feed instance paint normally. + const [inheritedThreadTaskId] = useState( + () => useThreadPanelStore.getState().openByChannel[channelId] ?? null, + ); + useEffect(() => { + if (inheritedThreadTaskId) { + useThreadPanelStore.getState().closeThread(channelId); + } + }, [channelId, inheritedThreadTaskId]); + const handleSuggestionSelect = useCallback( (prompt: string, mode?: string) => { composerRef.current?.applySuggestion(prompt, mode); @@ -317,7 +330,7 @@ export function WebsiteChannelHome({ channelId }: { channelId: string }) {
- {threadTaskId && ( + {threadTaskId && threadTaskId !== inheritedThreadTaskId && ( ({ + channels: [{ id: "personal-space", name: "me", path: "/me" }], + channelsLoading: false, + useLoops: vi.fn(() => ({ data: [], isLoading: false, isError: false })), +})); + +vi.mock("@posthog/ui/features/canvas/hooks/useChannels", () => ({ + useChannels: () => ({ + channels: mocks.channels, + isLoading: mocks.channelsLoading, + }), +})); +vi.mock("@posthog/ui/features/canvas/hooks/useChannelsLayout", () => ({ + useChannelsLayout: () => true, +})); +vi.mock("@posthog/ui/features/canvas/components/ChannelHeader", () => ({ + ChannelHeader: () =>
Personal space header
, +})); +vi.mock("@posthog/ui/hooks/useSetHeaderContent", () => ({ + useSetHeaderContent: () => {}, +})); +vi.mock("@posthog/ui/router/navigationBridge", () => ({ + navigateToNewLoop: vi.fn(), +})); +vi.mock("@posthog/ui/features/canvas/hooks/useOrgMembers", () => ({ + useOrgMembers: () => ({ + members: [], + isLoading: false, + isError: false, + isComplete: true, + }), +})); +vi.mock("@posthog/ui/features/loops/hooks/useLoops", () => ({ + useLoops: mocks.useLoops, + useLoopLimits: () => null, +})); +vi.mock("@posthog/ui/features/loops/components/LoopBuilderComposer", () => ({ + LoopBuilderComposer: () => null, +})); +vi.mock("@posthog/ui/features/loops/components/LoopFallbacks", () => ({ + LoopsEmptyNotice: () => null, + LoopsSkeleton: () =>
Loading loops
, +})); +vi.mock("@posthog/ui/features/loops/components/LoopRow", () => ({ + LoopRow: () => null, +})); +vi.mock("@posthog/ui/features/loops/components/LoopsEmptyState", () => ({ + LoopsEmptyState: () => null, +})); +vi.mock("@posthog/ui/features/loops/components/LoopTemplatesSection", () => ({ + LoopTemplatesSection: () => null, +})); +vi.mock("@posthog/ui/features/canvas/hooks/useTaskChannels", () => ({ + PERSONAL_CHANNEL_NAME: "me", +})); +vi.mock("@posthog/ui/features/loops/components/LoopsListView", () => ({ + LoopsListView: ({ headerContent }: { headerContent?: ReactNode }) => ( +
+ {headerContent} + Project loops registry +
+ ), +})); + +import { WebsiteChannelLoops } from "./WebsiteChannelLoops"; + +describe("WebsiteChannelLoops", () => { + beforeEach(() => { + mocks.channels = [{ id: "personal-space", name: "me", path: "/me" }]; + mocks.channelsLoading = false; + mocks.useLoops.mockClear(); + }); + + it("shows the project loops registry in the Personal space", () => { + render(); + + expect(screen.getByText("Project loops registry")).toBeInTheDocument(); + expect(screen.getByText("Personal space header")).toBeInTheDocument(); + }); + + it("waits for the Personal space to resolve before choosing a list", () => { + mocks.channels = []; + mocks.channelsLoading = true; + + render(); + + expect(screen.getByText("Loading loops")).toBeInTheDocument(); + expect( + screen.queryByText("Project loops registry"), + ).not.toBeInTheDocument(); + expect(mocks.useLoops).not.toHaveBeenCalled(); + }); +}); diff --git a/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelLoops.tsx b/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelLoops.tsx index 44efad88aa91..c9e789ea0641 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelLoops.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/WebsiteChannelLoops.tsx @@ -14,7 +14,7 @@ import { } from "@posthog/ui/primitives/PageHeader"; import { navigateToNewLoop } from "@posthog/ui/router/navigationBridge"; import { Flex, Heading, Text } from "@radix-ui/themes"; -import { useMemo } from "react"; +import { type ReactNode, useMemo } from "react"; import { LoopBuilderComposer } from "../../loops/components/LoopBuilderComposer"; import { LoopsEmptyNotice, @@ -22,6 +22,7 @@ import { } from "../../loops/components/LoopFallbacks"; import { LoopRow } from "../../loops/components/LoopRow"; import { LoopsEmptyState } from "../../loops/components/LoopsEmptyState"; +import { LoopsListView } from "../../loops/components/LoopsListView"; import { LoopTemplatesSection } from "../../loops/components/LoopTemplatesSection"; import { useLoopLimits, useLoops } from "../../loops/hooks/useLoops"; import { useLoopDraftStore } from "../../loops/loopDraftStore"; @@ -56,6 +57,52 @@ function contextQuickStarts(name: string): { label: string; prompt: string }[] { * composer pinned at the bottom), but the build surface is tuned to automations that feed * this context. `channelId` is the desktop folder id, matching `context_target.folder_id`. */ export function WebsiteChannelLoops({ channelId }: { channelId: string }) { + const { channels, isLoading } = useChannels(); + const channel = channels.find((candidate) => candidate.id === channelId); + const headerContent = useMemo( + () => , + [channelId], + ); + + // Don't mount the scoped scene while the route's space is unresolved. In + // particular, that would flash a raw-id empty state for Personal before the + // channel query identifies it as the project-level loops registry. + if (isLoading && !channel) { + return ; + } + + // The Personal space is the project-level home for loops in the spaces + // layout. API-created and other unattached loops have no context_target, so + // rendering the space-scoped list here incorrectly produces the global + // "Create your first loop" empty state while those loops already exist. + if (channel?.name === PERSONAL_CHANNEL_NAME) { + return ; + } + + return ( + + ); +} + +function ChannelLoopsLoading({ headerContent }: { headerContent: ReactNode }) { + useSetHeaderContent(headerContent); + return ( +
+ +
+ ); +} + +function SpaceAttachedLoops({ + channelId, + contextName, +}: { + channelId: string; + contextName: string; +}) { const { data: loops, isLoading, isError } = useLoops(); const spacesLayout = useChannelsLayout(); const limits = useLoopLimits(); @@ -63,10 +110,6 @@ export function WebsiteChannelLoops({ channelId }: { channelId: string }) { limits?.atLimit === true ? `You've reached the limit of ${limits.max} loops for this project. Delete one to add another.` : null; - const { channels } = useChannels(); - const channel = channels.find((c) => c.id === channelId); - const contextName = channel?.name ?? channelId; - const isPersonal = contextName === PERSONAL_CHANNEL_NAME; useSetHeaderContent( useMemo( @@ -113,7 +156,7 @@ export function WebsiteChannelLoops({ channelId }: { channelId: string }) { navigateToNewLoop(); }; - const title = isPersonal ? "Loops" : `Automate #${contextName}`; + const title = `Automate #${contextName}`; const description = "Put your work on autopilot. Loops run on a schedule, on an API call, or when something happens on GitHub. You can finally close the laptop!"; const createButton = ( @@ -214,9 +257,7 @@ export function WebsiteChannelLoops({ channelId }: { channelId: string }) { ) : ( - + )} diff --git a/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.test.tsx b/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.test.tsx index 27401260b11b..2d01ef485c79 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.test.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.test.tsx @@ -1,4 +1,4 @@ -import { CubeIcon, HashIcon, LockSimpleIcon } from "@phosphor-icons/react"; +import { HashIcon, LockSimpleIcon } from "@phosphor-icons/react"; import type { ReactElement } from "react"; import { describe, expect, it } from "vitest"; import { channelGlyph, isPrivateChannel } from "./channelGlyph"; @@ -23,7 +23,6 @@ describe("isPrivateChannel", () => { describe("channelGlyph", () => { it.each([ ["channel", false, HashIcon], - ["space", true, CubeIcon], ["private space", true, LockSimpleIcon], ])("renders the %s glyph", (_, space, expectedIcon) => { const name = expectedIcon === LockSimpleIcon ? "me" : "engineering"; @@ -31,4 +30,10 @@ describe("channelGlyph", () => { expect(glyph.type).toBe(expectedIcon); }); + + // A shared space carries no mark at all: the cube said nothing the name + // didn't, and only the private one is worth calling out. + it("gives a shared space no glyph", () => { + expect(channelGlyph("engineering", { space: true })).toBeNull(); + }); }); diff --git a/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.tsx b/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.tsx index 8e790e3548e7..f571d6606508 100644 --- a/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.tsx +++ b/products/desktop/packages/ui/src/features/canvas/components/channelGlyph.tsx @@ -1,5 +1,4 @@ import { - CubeIcon, HashIcon, type IconWeight, LockSimpleIcon, @@ -24,8 +23,13 @@ export function isPrivateChannel(channelName: string | undefined): boolean { } /** - * A channel's leading glyph: a lock when it's private, otherwise a cube for the - * Spaces layout or a hash for legacy Channels. + * A channel's leading glyph: a lock when it's private, a hash under the legacy + * Channels layout, and nothing at all for a space. + * + * Spaces dropped their cube because it said nothing the name didn't — a column + * of identical marks is noise, and the only thing worth calling out in that list + * is the one space that isn't shared. The hash stays where it still separates a + * channel from the other things in that tree. */ export function channelGlyph( channelName: string | undefined, @@ -36,11 +40,8 @@ export function channelGlyph( space?: boolean; }, ): ReactNode { - const Icon = isPrivateChannel(channelName) - ? LockSimpleIcon - : opts?.space - ? CubeIcon - : HashIcon; + if (!isPrivateChannel(channelName) && opts?.space) return null; + const Icon = isPrivateChannel(channelName) ? LockSimpleIcon : HashIcon; return ( { "./ensurePersonalChannel" ); const create = vi.fn( - () => new Promise((r) => setTimeout(() => r(channel("1")), 5)), + () => + new Promise((r) => setTimeout(() => r(channel("1")), 5)), ); const [a, b] = await Promise.all([ensure([], create), ensure([], create)]); @@ -69,7 +72,7 @@ it("lets a later caller retry after a failed create", async () => { "./ensurePersonalChannel" ); const create = vi - .fn<() => Promise>() + .fn<() => Promise>() .mockRejectedValueOnce(new Error("offline")) .mockResolvedValueOnce(channel("1")); @@ -77,3 +80,15 @@ it("lets a later caller retry after a failed create", async () => { await expect(ensure([], create)).resolves.toEqual(channel("1")); expect(create).toHaveBeenCalledTimes(2); }); + +it("does not share a created folder between scopes", async () => { + const { ensurePersonalChannel: ensure } = await import( + "./ensurePersonalChannel" + ); + const createFirst = vi.fn(async () => channel("1")); + const createSecond = vi.fn(async () => channel("2")); + + await expect(ensure([], createFirst, {})).resolves.toEqual(channel("1")); + await expect(ensure([], createSecond, {})).resolves.toEqual(channel("2")); + expect(createSecond).toHaveBeenCalledOnce(); +}); diff --git a/products/desktop/packages/ui/src/features/canvas/ensurePersonalChannel.ts b/products/desktop/packages/ui/src/features/canvas/ensurePersonalChannel.ts index 8d1fe49dc919..9dcb8cac56ae 100644 --- a/products/desktop/packages/ui/src/features/canvas/ensurePersonalChannel.ts +++ b/products/desktop/packages/ui/src/features/canvas/ensurePersonalChannel.ts @@ -1,19 +1,34 @@ -import type { Channel } from "@posthog/ui/features/canvas/hooks/useChannels"; +import type { PostHogAPIClient } from "@posthog/api-client/posthog-client"; import { PERSONAL_CHANNEL_NAME } from "@posthog/ui/features/canvas/hooks/useTaskChannels"; +export interface PersonalChannel { + id: string; + name: string; +} + +export type PersonalChannelClient = Pick< + PostHogAPIClient, + "createDesktopFileSystemChannel" | "getDesktopFileSystemChannels" +>; + // The "me" folder is provisioned on first use, and folder creation is not // server-side idempotent by path — so two callers racing before the first // create lands in the channels cache would each make their own "me". The entry // points are trivially concurrent (Cmd+T's new tab, the sidebar row, its "+" // menu), so they share one in-flight create rather than guarding separately: // per-caller guards would still race each other. -let inFlight: Promise | null = null; +interface PersonalChannelState { + inFlight: Promise | null; + created: PersonalChannel | null; +} + +const sharedScope = {}; +const stateByScope = new WeakMap(); // The in-flight promise alone isn't enough: it settles the moment the POST // returns, but callers pass the `channels` from their last render, which hasn't // re-rendered with the seeded cache yet. A click landing in that gap sees // neither an existing "me" nor an in-flight create, and makes a second one. // Remember what was created until the list catches up. -let created: Channel | null = null; /** * The user's "me" folder, creating it once if it doesn't exist yet. Concurrent @@ -21,26 +36,47 @@ let created: Channel | null = null; * messaging. */ export async function ensurePersonalChannel( - channels: readonly Channel[], - createChannel: (name: string) => Promise, -): Promise { + channels: readonly PersonalChannel[], + createChannel: (name: string) => Promise, + scope: object = sharedScope, +): Promise { + const state = stateByScope.get(scope) ?? { inFlight: null, created: null }; + stateByScope.set(scope, state); const existing = channels.find((c) => c.name === PERSONAL_CHANNEL_NAME); if (existing) { // The list is authoritative once it carries the folder: drop the memo, so a // deleted-then-recreated "me" resolves fresh rather than to a dead id. - created = null; + state.created = null; return existing; } - if (created) return created; - if (!inFlight) { - inFlight = createChannel(PERSONAL_CHANNEL_NAME) + if (state.created) return state.created; + if (!state.inFlight) { + state.inFlight = createChannel(PERSONAL_CHANNEL_NAME) .then((channel) => { - created = channel; + state.created = channel; return channel; }) .finally(() => { - inFlight = null; + state.inFlight = null; }); } - return inFlight; + return state.inFlight; +} + +export async function ensurePersonalChannelFromClient( + client: PersonalChannelClient, +): Promise { + const toPersonalChannel = ({ id, path }: { id: string; path: string }) => ({ + id, + name: path.replace(/^\/+/, ""), + }); + const channels = (await client.getDesktopFileSystemChannels()) + .filter((channel) => channel.type === "folder") + .map(toPersonalChannel); + return await ensurePersonalChannel( + channels, + async (name) => + toPersonalChannel(await client.createDesktopFileSystemChannel(name)), + client, + ); } diff --git a/products/desktop/packages/ui/src/features/canvas/hooks/useChannelItems.tsx b/products/desktop/packages/ui/src/features/canvas/hooks/useChannelItems.tsx index eb2c022a1ede..721a2f116580 100644 --- a/products/desktop/packages/ui/src/features/canvas/hooks/useChannelItems.tsx +++ b/products/desktop/packages/ui/src/features/canvas/hooks/useChannelItems.tsx @@ -8,6 +8,7 @@ import { useArchiveTask } from "@posthog/ui/features/archive/useArchiveTask"; import { useOptionalAuthenticatedClient } from "@posthog/ui/features/auth/authClient"; import { useCurrentUser } from "@posthog/ui/features/auth/useCurrentUser"; import type { ChannelItemActions } from "@posthog/ui/features/canvas/components/ChannelItemRow"; +import { deleteCanvasWithUndo } from "@posthog/ui/features/canvas/deleteCanvasWithUndo"; import { useChannelFeed } from "@posthog/ui/features/canvas/hooks/useChannelFeed"; import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; import { useChannelTasks } from "@posthog/ui/features/canvas/hooks/useChannelTasks"; @@ -68,7 +69,8 @@ export function useChannelItems(channelId: string): { const archivedTaskIds = useArchivedTaskIds(); const { pinnedTaskIds, togglePin } = usePinnedTasks(); const { archiveTask } = useArchiveTask({ navigateSpace: "website" }); - const { setPinned: setCanvasPinned } = useDashboardMutations(); + const { setPinned: setCanvasPinned, invalidateDashboards } = + useDashboardMutations(); const client = useOptionalAuthenticatedClient(); const { data: currentUser, isLoading: viewerLoading } = useCurrentUser({ client, @@ -143,8 +145,28 @@ export function useChannelItems(channelId: string): { archive: (item) => { void archiveTask({ taskId: item.id }); }, + // Canvases only, and through the shared undo window: the row disappears at + // once and the host isn't told until the toast expires, so an accidental + // delete costs nothing. + remove: (item) => { + if (item.kind !== "canvas") return; + deleteCanvasWithUndo({ + dashboardId: item.id, + channelId, + name: item.title, + surface: "sidebar", + invalidate: invalidateDashboards, + }); + }, }), - [channelId, navigate, setCanvasPinned, togglePin, archiveTask], + [ + channelId, + navigate, + setCanvasPinned, + togglePin, + archiveTask, + invalidateDashboards, + ], ); // A channel that isn't in the list will never resolve, so stop reporting diff --git a/products/desktop/packages/ui/src/features/canvas/hooks/useChannelTaskStatus.ts b/products/desktop/packages/ui/src/features/canvas/hooks/useChannelTaskStatus.ts new file mode 100644 index 000000000000..f92790e58244 --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/hooks/useChannelTaskStatus.ts @@ -0,0 +1,49 @@ +import type { ChannelItemModel } from "@posthog/core/canvas/channelItems"; +import { useChannelTaskData } from "@posthog/ui/features/canvas/hooks/useChannelTaskData"; +import type { TaskStatusInput } from "@posthog/ui/features/sidebar/components/items/taskStatusVocabulary"; +import { useTaskPrStatus } from "@posthog/ui/features/sidebar/useTaskPrStatus"; +import { useWorkspace } from "@posthog/ui/features/workspace/useWorkspace"; + +/** + * The state behind a channel row's status dot and badges, or `null` for a canvas + * (which has no run to report). + * + * Assembled per row, the same way the Code sidebar's `TaskRow` does it, because + * that's the only place the inputs exist together: the derived flags come from + * renderer state (live session, workspace, viewed timestamps) and the PR state + * from a per-task query, so none of it can be baked into the item list in core. + * Keeping the composition in a hook rather than in the row leaves the row a + * component that renders, and gives tests one module to stub. + */ +export function useChannelTaskStatus( + item: ChannelItemModel, +): TaskStatusInput | null { + const task = item.task ?? undefined; + const taskData = useChannelTaskData(task); + const workspace = useWorkspace(task?.id); + const { prState, hasDiff } = useTaskPrStatus({ + id: task?.id ?? "", + cloudPrUrl: taskData?.cloudPrUrl ?? null, + taskRunEnvironment: taskData?.taskRunEnvironment ?? null, + }); + + if (!taskData) return null; + return { + workspaceMode: + workspace?.mode ?? + (taskData.taskRunEnvironment === "cloud" ? "cloud" : undefined), + isGenerating: taskData.isGenerating, + isUnread: taskData.isUnread, + isPinned: taskData.isPinned, + isSuspended: taskData.isSuspended, + needsPermission: taskData.needsPermission, + taskRunStatus: taskData.taskRunStatus, + originProduct: taskData.originProduct, + slackThreadUrl: taskData.slackThreadUrl, + prState, + hasDiff, + // The url is the early signal: a cloud run writes it the moment it opens the + // PR, long before (or without ever) resolving the PR's state. + prUrl: taskData.cloudPrUrl, + }; +} diff --git a/products/desktop/packages/ui/src/features/canvas/hooks/useFileTaskToChannel.ts b/products/desktop/packages/ui/src/features/canvas/hooks/useFileTaskToChannel.ts new file mode 100644 index 000000000000..ed6f88d4b8ce --- /dev/null +++ b/products/desktop/packages/ui/src/features/canvas/hooks/useFileTaskToChannel.ts @@ -0,0 +1,36 @@ +import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; +import { useChannelTaskMutations } from "@posthog/ui/features/canvas/hooks/useChannelTasks"; +import { toast } from "@posthog/ui/primitives/toast"; +import { useCallback } from "react"; + +/** + * Files a task to a space and reports the outcome, naming the space in the + * success toast. Extracted so the row menu and the native context menu file + * tasks the same way — filing is a mutation plus the two toasts that make it + * legible, and duplicating that is how the two paths drift. + */ +export function useFileTaskToChannel(): ( + channelId: string, + taskId: string, + taskTitle: string, +) => Promise { + const { fileTask } = useChannelTaskMutations(); + const { channels } = useChannels(); + + return useCallback( + async (channelId: string, taskId: string, taskTitle: string) => { + try { + await fileTask(channelId, taskId, taskTitle); + const channelName = channels.find( + (channel) => channel.id === channelId, + )?.name; + toast.success(channelName ? `Filed to ${channelName}` : "Task filed"); + } catch (error) { + toast.error("Couldn't file task", { + description: error instanceof Error ? error.message : String(error), + }); + } + }, + [channels, fileTask], + ); +} diff --git a/products/desktop/packages/ui/src/features/inbox/components/AgentRunDetail.tsx b/products/desktop/packages/ui/src/features/inbox/components/AgentRunDetail.tsx index c8751d0306fc..4f32aea5e9e1 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/AgentRunDetail.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/AgentRunDetail.tsx @@ -110,7 +110,7 @@ function RunOutputWidget({ report }: { report: SignalReport }) { Research couldn't complete – check the task log below for the error. - The Responder may retry automatically. + The agent may retry automatically. @@ -123,7 +123,7 @@ function RunOutputWidget({ report }: { report: SignalReport }) { content={report.summary} fallback={ report.status === "in_progress" - ? "The Responder is investigating – partial findings will appear here as they land." + ? "The agent is investigating – partial signals will appear here as they land." : "Queued for research." } variant="detail" @@ -325,7 +325,7 @@ function AgentRunDetailContent({ report }: { report: SignalReport }) { <> - {signals.length} finding{signals.length === 1 ? "" : "s"} + {signals.length} signal{signals.length === 1 ? "" : "s"} )} @@ -412,7 +412,7 @@ function AgentRunDetailContent({ report }: { report: SignalReport }) { title="Evidence so far" rightSlot={ - {signals.length || report.signal_count} finding + {signals.length || report.signal_count} signal {(signals.length || report.signal_count) === 1 ? "" : "s"} } diff --git a/products/desktop/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx b/products/desktop/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx index 5dc82aab4d0e..32af3af5d91b 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/ConfigureAgentsSection.tsx @@ -124,7 +124,7 @@ export function ConfigureAgentsSection() { Scheduled agents that sweep this project on a cadence and emit - findings to your inbox.{" "} + signals to your inbox.{" "} {/* Placeholder docs link until a dedicated scouts page exists. */} {isLoading ? ( @@ -207,7 +207,7 @@ export function ConfigureAgentsSection() { diff --git a/products/desktop/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx b/products/desktop/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx index bcaacd6c003d..e122dcd0ae8c 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/InboxDetailFrame.tsx @@ -137,7 +137,7 @@ export function InboxDetailFrame({ {evidenceCount > 0 && ( <> - {evidenceCount} finding{evidenceCount === 1 ? "" : "s"} + {evidenceCount} signal{evidenceCount === 1 ? "" : "s"} @@ -151,7 +151,7 @@ export function InboxDetailFrame({ )} @@ -181,7 +181,7 @@ export function InboxDetailFrame({ @@ -196,7 +196,7 @@ export function InboxDetailFrame({ title={evidenceSection.title} rightSlot={ - {evidenceCount} finding + {evidenceCount} signal {evidenceCount === 1 ? "" : "s"} } diff --git a/products/desktop/packages/ui/src/features/inbox/components/PullRequestsTab.tsx b/products/desktop/packages/ui/src/features/inbox/components/PullRequestsTab.tsx index e29d53bdc934..1d33402ca507 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/PullRequestsTab.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/PullRequestsTab.tsx @@ -24,7 +24,7 @@ export function PullRequestsTab() { entireProjectTitle: "No pull requests in the project right now", teammateTitle: "No pull requests for this reviewer right now", description: - "When a Responder ships a code change, the PR draft lands here for you to review and publish.", + "When an agent ships a code change, the PR draft lands here for you to review and publish.", }} /> ); diff --git a/products/desktop/packages/ui/src/features/inbox/components/ReportCard.tsx b/products/desktop/packages/ui/src/features/inbox/components/ReportCard.tsx index a58955c4afea..3cdfbbe2d93b 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/ReportCard.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/ReportCard.tsx @@ -323,7 +323,7 @@ export function ReportCard(props: ReportCardProps) { > - {report.signal_count} finding + {report.signal_count} signal {report.signal_count !== 1 ? "s" : ""} diff --git a/products/desktop/packages/ui/src/features/inbox/components/ReportsTab.tsx b/products/desktop/packages/ui/src/features/inbox/components/ReportsTab.tsx index 1b531a0172e1..c33746e8a32f 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/ReportsTab.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/ReportsTab.tsx @@ -15,7 +15,7 @@ export function ReportsTab() { entireProjectTitle: "No reports in the project yet", teammateTitle: "No reports for this reviewer yet", description: - "Reports are what Responders surface when there's something worth your judgment but no clean code change to draft.", + "Reports are what agents surface when there's something worth your judgment but no clean code change to draft.", }} /> ); diff --git a/products/desktop/packages/ui/src/features/inbox/components/RunsTab.tsx b/products/desktop/packages/ui/src/features/inbox/components/RunsTab.tsx index af45dab0d940..8e4b41279f2e 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/RunsTab.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/RunsTab.tsx @@ -95,10 +95,10 @@ export function RunsTab() { {scope === INBOX_SCOPE_FOR_YOU - ? "No Responders are working on something for you right now" + ? "No agents are working on something for you right now" : scope === INBOX_SCOPE_ENTIRE_PROJECT - ? "No Responders are working on anything in the project right now" - : "No Responders are working on something for this reviewer right now"} + ? "No agents are working on anything in the project right now" + : "No agents are working on something for this reviewer right now"} When Self-driving kicks one off, you'll see the live run land here diff --git a/products/desktop/packages/ui/src/features/inbox/components/utils/ExplainedDismissOptionLabels.tsx b/products/desktop/packages/ui/src/features/inbox/components/utils/ExplainedDismissOptionLabels.tsx index 5da50a8adfbb..203a3a53822b 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/utils/ExplainedDismissOptionLabels.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/utils/ExplainedDismissOptionLabels.tsx @@ -4,10 +4,10 @@ import { RadioGroup, Tooltip } from "@radix-ui/themes"; import type { ReactNode } from "react"; const PAUSE_OPTION_TOOLTIP = - "Snoozes this report: it briefly leaves your inbox while more context is gathered, and it can come back if new findings match."; + "Snoozes this report: it briefly leaves your inbox while more context is gathered, and it can come back if new signals match."; const SUPPRESS_OPTION_TOOLTIP = - "Archives permanently: the report leaves your inbox and matching findings will not surface it again. Your reason is saved with the report."; + "Archives permanently: the report leaves your inbox and matching signals will not surface it again. Your reason is saved with the report."; function dismissReasonOptionDomId(value: DismissalReasonOptionValue): string { return `dismiss-report-dialog-reason-${value}`; diff --git a/products/desktop/packages/ui/src/features/inbox/components/utils/SignalReportStatusBadge.tsx b/products/desktop/packages/ui/src/features/inbox/components/utils/SignalReportStatusBadge.tsx index 0acdd3e73bdf..9f72d0680e74 100644 --- a/products/desktop/packages/ui/src/features/inbox/components/utils/SignalReportStatusBadge.tsx +++ b/products/desktop/packages/ui/src/features/inbox/components/utils/SignalReportStatusBadge.tsx @@ -8,10 +8,10 @@ const STATUS_TOOLTIPS: Record = { resolved: "This report is resolved — its implementation pull request merged.", pending_input: "This report needs human input in PostHog before it can proceed.", - in_progress: "An AI agent is actively researching this report's findings.", + in_progress: "An AI agent is actively researching this report's signals.", candidate: "Queued for research. An agent will pick this up shortly.", potential: - "Gathering findings. The report will be queued once enough evidence accumulates.", + "Gathering signals. The report will be queued once enough evidence accumulates.", failed: "Research failed. The report may be retried automatically.", suppressed: "This report has been suppressed and is out of your inbox.", deleted: "This report has been deleted.", diff --git a/products/desktop/packages/ui/src/features/loops/components/LoopsListView.tsx b/products/desktop/packages/ui/src/features/loops/components/LoopsListView.tsx index a265fb6d51eb..e9f4be9a66e8 100644 --- a/products/desktop/packages/ui/src/features/loops/components/LoopsListView.tsx +++ b/products/desktop/packages/ui/src/features/loops/components/LoopsListView.tsx @@ -26,7 +26,7 @@ import { } from "@posthog/ui/router/navigationBridge"; import { track } from "@posthog/ui/shell/analytics"; import { Flex, Text } from "@radix-ui/themes"; -import { useEffect, useRef, useState } from "react"; +import { type ReactNode, useEffect, useRef, useState } from "react"; import { useLoopBuilderSessions } from "../hooks/useLoopBuilderSessions"; import { useLoopLimits, useLoops } from "../hooks/useLoops"; import { @@ -72,7 +72,11 @@ function startLoopFromTemplate(template: LoopTemplate): void { navigateToNewLoop(); } -export function LoopsListView() { +export function LoopsListView({ + headerContent = null, +}: { + headerContent?: ReactNode; +}) { const { data: loops, isLoading, isError, error } = useLoops(); const authenticatedClient = useOptionalAuthenticatedClient(); const { @@ -91,9 +95,10 @@ export function LoopsListView() { listError = currentUserQueryError; } - // The page names itself (in-page header / title block), so it pushes no - // breadcrumb row — only a space-attached loop scene has a parent to show. - useSetHeaderContent(null); + // The standalone page names itself in-page and has no breadcrumb. When the + // registry is hosted inside a space, its caller supplies that navigation + // context instead. + useSetHeaderContent(headerContent); const { sessions: builderSessions, isSettled: builderSessionsSettled } = useLoopBuilderSessions(); diff --git a/products/desktop/packages/ui/src/features/mcp-gateway/components/McpGatewayView.tsx b/products/desktop/packages/ui/src/features/mcp-gateway/components/McpGatewayView.tsx new file mode 100644 index 000000000000..5403d1b4ca5d --- /dev/null +++ b/products/desktop/packages/ui/src/features/mcp-gateway/components/McpGatewayView.tsx @@ -0,0 +1,139 @@ +import { GatewayAddServer } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayAddServer"; +import { GatewayAgentDetail } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayAgentDetail"; +import { GatewayAuditLog } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayAuditLog"; +import { GatewayMemberDetail } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayMemberDetail"; +import { GatewayRail } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayRail"; +import { GatewayServerDetail } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayServerDetail"; +import { GatewayServersHome } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayServersHome"; +import { GatewayTeamSettings } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayTeamSettings"; +import { GatewayTeamView } from "@posthog/ui/features/mcp-gateway/components/parts/GatewayTeamView"; +import { + type GatewayRoute, + isRouteAllowed, +} from "@posthog/ui/features/mcp-gateway/gatewayRoute"; +import { useGatewayConfig } from "@posthog/ui/features/mcp-gateway/hooks/useGatewayConfig"; +import { useGatewayServers } from "@posthog/ui/features/mcp-gateway/hooks/useGatewayServers"; +import { useServiceAccounts } from "@posthog/ui/features/mcp-gateway/hooks/useServiceAccounts"; +import { DotPatternBackground } from "@posthog/ui/primitives/DotPatternBackground"; +import { Box, Flex, ScrollArea } from "@radix-ui/themes"; +import { useQueryClient } from "@tanstack/react-query"; +import { useEffect, useState } from "react"; + +/** + * Team MCP gateway: one control plane for the servers a team runs, who can + * reach them (members and agent service accounts), per-tool policies per + * scope, and the audit log. Renders behind the `mcp-gateway` flag in place of + * the per-user marketplace. + */ +export function McpGatewayView() { + const queryClient = useQueryClient(); + const [requestedRoute, setRoute] = useState({ + view: "servers", + }); + + const { isAdmin, allowCustomServers, canManageAgentAccess, configLoading } = + useGatewayConfig(); + const canAddServers = isAdmin || allowCustomServers; + const gateway = useGatewayServers(); + const serviceAccounts = useServiceAccounts(); + + // Refresh gateway state when the window regains focus — connections and + // policies can change from the web app or another teammate meanwhile. + useEffect(() => { + const refresh = () => { + queryClient.invalidateQueries({ queryKey: ["mcp"] }); + }; + const onVisibility = () => { + if (document.visibilityState === "visible") refresh(); + }; + window.addEventListener("focus", refresh); + document.addEventListener("visibilitychange", onVisibility); + return () => { + window.removeEventListener("focus", refresh); + document.removeEventListener("visibilitychange", onVisibility); + }; + }, [queryClient]); + + // Role guard, applied at render: if the config resolves to a narrower role + // than the stored route needs, show the servers home instead. + const route: GatewayRoute = + configLoading || isRouteAllowed(requestedRoute, { isAdmin, canAddServers }) + ? requestedRoute + : { view: "servers" }; + + const mainContent = (() => { + switch (route.view) { + case "add": + return ( + + ); + case "server": + return ( + + ); + case "team": + return ; + case "agent": + return ( + + ); + case "member": + return ( + + ); + case "settings": + return ; + case "audit": + return ; + default: + return ( + + ); + } + })(); + + return ( + + + + + + + {mainContent} + + + + + ); +} diff --git a/products/desktop/packages/ui/src/features/mcp-gateway/components/parts/GatewayAddServer.test.tsx b/products/desktop/packages/ui/src/features/mcp-gateway/components/parts/GatewayAddServer.test.tsx new file mode 100644 index 000000000000..4249382e9de8 --- /dev/null +++ b/products/desktop/packages/ui/src/features/mcp-gateway/components/parts/GatewayAddServer.test.tsx @@ -0,0 +1,52 @@ +import type { McpServiceAccount } from "@posthog/api-client/posthog-client"; +import { Theme } from "@radix-ui/themes"; +import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock( + "@posthog/ui/features/mcp-gateway/hooks/useRegisterGatewayServer", + () => ({ + useRegisterGatewayServer: () => ({ + register: vi.fn(), + registerPending: false, + }), + }), +); + +import { GatewayAddServer } from "./GatewayAddServer"; + +const account = { + id: "agent-1", + name: "Support agent", + description: "", + handle: "support-agent", + status: "active", + token_mask: "", + server_ids: [], + last_active_at: null, + created_at: "2026-07-23T12:00:00Z", + updated_at: "2026-07-23T12:00:00Z", +} as McpServiceAccount; + +describe("GatewayAddServer", () => { + it("keeps team and agent sharing without offering shared credentials", () => { + render( + + + , + ); + + expect(screen.getByText("Enable for the whole team")).toBeInTheDocument(); + expect(screen.getByText("Share with agents")).toBeInTheDocument(); + expect(screen.getByText(account.name)).toBeInTheDocument(); + expect(screen.queryByText("One shared credential")).not.toBeInTheDocument(); + expect( + screen.queryByText("Allow personal connections"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/products/desktop/packages/ui/src/features/mcp-gateway/components/parts/GatewayAddServer.tsx b/products/desktop/packages/ui/src/features/mcp-gateway/components/parts/GatewayAddServer.tsx new file mode 100644 index 000000000000..fd3303195afc --- /dev/null +++ b/products/desktop/packages/ui/src/features/mcp-gateway/components/parts/GatewayAddServer.tsx @@ -0,0 +1,395 @@ +import { ArrowLeft, CaretRight, Check } from "@phosphor-icons/react"; +import type { McpServiceAccount } from "@posthog/api-client/posthog-client"; +import { + buildGatewayInstallRequest, + canSubmitGatewayServer, + GATEWAY_ADD_SERVER_DEFAULTS, + type GatewayAddServerValues, +} from "@posthog/core/mcp-gateway/gatewayAddServer"; +import { isValidMcpUrl } from "@posthog/core/mcp-servers/customServerForm"; +import { RobotAvatar } from "@posthog/ui/features/mcp-gateway/components/parts/avatars"; +import type { GatewayRoute } from "@posthog/ui/features/mcp-gateway/gatewayRoute"; +import { useRegisterGatewayServer } from "@posthog/ui/features/mcp-gateway/hooks/useRegisterGatewayServer"; +import { + Button, + Flex, + Heading, + Select, + Spinner, + Switch, + Text, + TextArea, + TextField, +} from "@radix-ui/themes"; +import { type FormEvent, useState } from "react"; + +interface GatewayAddServerProps { + isAdmin: boolean; + canManageAgentAccess: boolean; + accounts: McpServiceAccount[]; + onNavigate: (route: GatewayRoute) => void; +} + +/** Register a custom MCP server with the gateway. */ +export function GatewayAddServer({ + isAdmin, + canManageAgentAccess, + accounts, + onNavigate, +}: GatewayAddServerProps) { + const [values, setValues] = useState( + GATEWAY_ADD_SERVER_DEFAULTS, + ); + const [showKey, setShowKey] = useState(false); + const [optionalOpen, setOptionalOpen] = useState(false); + + const { register, registerPending } = useRegisterGatewayServer(); + + const set = ( + key: K, + value: GatewayAddServerValues[K], + ) => setValues((previous) => ({ ...previous, [key]: value })); + + const urlInvalid = values.url.trim() !== "" && !isValidMcpUrl(values.url); + const canSave = canSubmitGatewayServer(values); + + const submit = (event: FormEvent) => { + event.preventDefault(); + if (!canSave || registerPending) return; + const request = buildGatewayInstallRequest(values, { + isAdmin, + canManageAgentAccess, + }); + register( + { request }, + { + onSuccess: (result) => { + if (result.created) { + onNavigate({ view: "server", serverId: result.created.id }); + } + }, + }, + ); + }; + + return ( +
+ + + + + + + Add a custom server + + Register an MCP server with the gateway. Every call routes through + the gateway, so tool policies, approvals and the audit log apply + from the first request. + + + + + + + set("name", e.target.value)} + placeholder="e.g. Internal Wiki" + autoFocus + /> + + + set("url", e.target.value)} + placeholder="https://mcp.example.com/sse" + spellCheck={false} + className="font-mono" + /> + {urlInvalid && ( + + Enter a full URL, like https://mcp.example.com + + )} + + +