From 296fb3861c0eb89438c9c63ed1df0eb8fc995a19 Mon Sep 17 00:00:00 2001 From: Tom Owers Date: Wed, 9 Sep 2026 14:01:40 +0200 Subject: [PATCH] fix(orchestrator): carry the data-source next steps onto the outro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration program builds a pre-filled link per detected data source for the outro, but only the linear sequence asks a program for its outro data. The orchestrated sequence composes its own message from the drain result, so the links were built and then discarded on every orchestrated run — including the runs where the seeded warehouse step never connected anything, which is exactly when they are the only deterministic pointer the user has left. Adds `ProgramRun.buildOutroNextSteps`, a hook for a sequence that owns its own outro message: the sequence keeps the message, the program supplies the bullets. It is handed the runner-seeded task types that finished successfully, so the integration leaves the bullets off a run whose own warehouse step connected the sources — otherwise a successful run would end by asking the user to redo the work at a new-source form that collides with the source just created. The sequence stays ignorant of what any task type means. Branch: posthog/orchestrated-outro-next-steps Generated-By: PostHog Desktop Task-Id: 3db08a36-bd9a-4c3d-b384-d6070267db7c --- .../__tests__/completed-seeded-types.test.ts | 73 +++++++++++++++++++ .../orchestrator/orchestrator-runner.ts | 22 ++++++ src/lib/agent/runner/shared/types.ts | 20 +++++ .../__tests__/warehouse-suggestion.test.ts | 62 ++++++++++++++-- src/lib/programs/posthog-integration/index.ts | 24 +++++- 5 files changed, 194 insertions(+), 7 deletions(-) create mode 100644 src/lib/agent/runner/sequence/orchestrator/__tests__/completed-seeded-types.test.ts diff --git a/src/lib/agent/runner/sequence/orchestrator/__tests__/completed-seeded-types.test.ts b/src/lib/agent/runner/sequence/orchestrator/__tests__/completed-seeded-types.test.ts new file mode 100644 index 00000000..49e60ce3 --- /dev/null +++ b/src/lib/agent/runner/sequence/orchestrator/__tests__/completed-seeded-types.test.ts @@ -0,0 +1,73 @@ +/** + * Which seeded types the orchestrated outro reports as done. + * + * The outro's next-step bullets are what the user is left with when a seeded + * step did not carry out its work, so every terminal state other than `done` + * has to keep them — a declined step is the case they exist for. + */ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +vi.mock('@utils/analytics', () => ({ + analytics: { wizardCapture: vi.fn(), captureException: vi.fn() }, +})); + +import { + QueueStore, + SkipReason, +} from '@lib/agent/runner/sequence/orchestrator/queue'; +import { completedSeededTypes } from '@lib/agent/runner/sequence/orchestrator/orchestrator-runner'; + +describe('completedSeededTypes', () => { + let dir: string; + let store: QueueStore; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'seeded-outcome-test-')); + store = new QueueStore(dir, 'run-1'); + }); + + afterEach(() => fs.rmSync(dir, { recursive: true, force: true })); + + it('names a seeded type that completed', () => { + const warehouse = store.enqueue({ type: 'warehouse', optional: true }); + store.start(warehouse.id); + store.complete(warehouse.id); + + expect(completedSeededTypes(store, [warehouse])).toEqual(['warehouse']); + }); + + it('leaves out a declined seeded type', () => { + const warehouse = store.enqueue({ type: 'warehouse', optional: true }); + store.skip(warehouse.id, SkipReason.UserDeclined); + + expect(completedSeededTypes(store, [warehouse])).toEqual([]); + }); + + it('leaves out a seeded type the agent reported not needed', () => { + const warehouse = store.enqueue({ type: 'warehouse', optional: true }); + store.start(warehouse.id); + store.skip(warehouse.id, SkipReason.AgentNotNeeded); + + expect(completedSeededTypes(store, [warehouse])).toEqual([]); + }); + + it('leaves out a failed seeded type', () => { + const warehouse = store.enqueue({ type: 'warehouse', optional: true }); + store.start(warehouse.id); + store.fail(warehouse.id, { type: 'self-reported', message: 'x' }); + + expect(completedSeededTypes(store, [warehouse])).toEqual([]); + }); + + it('ignores tasks the wizard did not seed', () => { + const warehouse = store.enqueue({ type: 'warehouse', optional: true }); + const install = store.enqueue({ type: 'install' }); + store.start(install.id); + store.complete(install.id); + store.skip(warehouse.id, SkipReason.UserDeclined); + + expect(completedSeededTypes(store, [warehouse])).toEqual([]); + }); +}); diff --git a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts index d6a75a59..0254728e 100644 --- a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -246,6 +246,23 @@ export async function offerSeededTask( } } +/** + * Which runner-seeded task types finished successfully. + * + * Handed to the program's `buildOutroNextSteps` so it can drop a next step its + * own seeded task already carried out. Only `Done` counts: a skipped, declined + * or failed step left the work undone, and that is exactly when the outro + * bullet pointing at the app is the one thing the user still needs. + */ +export function completedSeededTypes( + store: QueueStore, + seededTasks: readonly QueuedTask[], +): string[] { + return seededTasks + .filter((task) => store.get(task.id)?.status === TaskStatus.Done) + .map((task) => task.type); +} + /** One seeded task's answer to its notice, taken once, at seed time. */ export interface SeededConsent { keep: boolean; @@ -1188,6 +1205,11 @@ export async function runOrchestrator( ? `⚠ Build conflict: ${conflict}\nFull details are in the setup report.` : undefined, docsUrl: 'https://posthog.com/docs/ai-engineering/ai-wizard', + nextSteps: config.buildOutroNextSteps?.( + session, + boot.credentials, + completedSeededTypes(store, seededTasks), + ), }); getUI().outro(message); await analytics.shutdown('success'); diff --git a/src/lib/agent/runner/shared/types.ts b/src/lib/agent/runner/shared/types.ts index 2c9e4b14..4f4292f8 100644 --- a/src/lib/agent/runner/shared/types.ts +++ b/src/lib/agent/runner/shared/types.ts @@ -60,6 +60,26 @@ export interface ProgramRun { session: WizardSession, credentials: Credentials, ) => WizardSession['outroData']; + /** + * Outro bullets for a sequence that composes its own outro data. + * + * `buildOutroData` is the linear sequence's seam: it hands the program the + * whole outro. The orchestrated sequence cannot, because its message is the + * drain's result — how many steps ran, what was skipped, which conflict the + * review step left. So a program with next steps to offer had nowhere to put + * them there, and the integration's data-source links were built and then + * dropped on every orchestrated run. This hook keeps the message with the + * sequence and the bullets with the program. + * + * `completedSeededTypes` names the runner-seeded task types that finished + * successfully, so a program can leave out a step its own seeded task + * already did — the sequence stays ignorant of what any type means. + */ + buildOutroNextSteps?: ( + session: WizardSession, + credentials: Credentials, + completedSeededTypes: readonly string[], + ) => { heading: string; items: string[] } | undefined; /** * Per-run cap on `wizard_ask` invocations. Defaults to 10. The 4th call * always returns a "batch your questions" error regardless of the cap. diff --git a/src/lib/programs/__tests__/warehouse-suggestion.test.ts b/src/lib/programs/__tests__/warehouse-suggestion.test.ts index 97257b22..de7a39b9 100644 --- a/src/lib/programs/__tests__/warehouse-suggestion.test.ts +++ b/src/lib/programs/__tests__/warehouse-suggestion.test.ts @@ -1,11 +1,16 @@ /** * Data-warehouse-source suggestion in the default integration flow. * - * The flow detects connectable sources and *points at* them — it does not - * connect them. These tests pin the three properties that matter: the outro - * hands over a link that opens the right source's form, projects with no - * detected source see a byte-identical flow, and the suggestion never turns - * into an inline run. + * These tests pin the properties that matter: the outro hands over a link that + * opens the right source's form, projects with no detected source see a + * byte-identical flow, and the suggestion never turns into an inline run. + * + * The links reach the user two ways, because the two sequences build the outro + * differently: the linear one asks the program for the whole thing + * (`buildOutroData`), while the orchestrated one composes its own message from + * the drain and takes only the bullets (`buildOutroNextSteps`). The second is + * the sequence that seeds the warehouse step, so it is also the one that can + * say the run already connected the sources. */ import { posthogIntegrationConfig } from '@lib/programs/posthog-integration/index'; @@ -209,3 +214,50 @@ describe('flow shape', () => { expect(POSTHOG_INTEGRATION_PROGRAM.some((s) => s.run)).toBe(false); }); }); + +describe('orchestrated outro suggestion', () => { + const nextSteps = async ( + session: WizardSession, + completedSeededTypes: readonly string[], + ) => { + const runDef = await resolveRun(session); + return runDef.buildOutroNextSteps!( + session, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + CREDENTIALS as any, + completedSeededTypes, + ); + }; + + it('carries the same links the linear outro does', async () => { + const s = sessionWith([POSTGRES, STRIPE]); + + const text = (await nextSteps(s, []))!.items.join('\n'); + + expect(text).toContain( + 'https://us.posthog.com/project/1/data-warehouse/new-source?kind=postgres', + ); + expect(text).toContain('kind=stripe'); + expect(text).toContain('npx @posthog/wizard warehouse'); + }); + + it('still carries them when the seeded step did not connect the sources', async () => { + const s = sessionWith([POSTGRES]); + + // A declined, skipped or failed warehouse step leaves the sources + // unconnected, which is the case these bullets exist for. + expect((await nextSteps(s, ['install']))!.items.join('\n')).toContain( + 'kind=postgres', + ); + }); + + it('offers nothing once the seeded warehouse step connected them', async () => { + const s = sessionWith([POSTGRES]); + + expect(await nextSteps(s, ['warehouse'])).toBeUndefined(); + }); + + it('offers nothing when nothing was detected', async () => { + expect(await nextSteps(sessionWith([]), [])).toBeUndefined(); + }); +}); diff --git a/src/lib/programs/posthog-integration/index.ts b/src/lib/programs/posthog-integration/index.ts index 7a90f313..67220611 100644 --- a/src/lib/programs/posthog-integration/index.ts +++ b/src/lib/programs/posthog-integration/index.ts @@ -32,6 +32,9 @@ const DASHBOARD_DEEP_LINK_KEY = 'dashboardDeepLink'; const WAREHOUSE_SOURCES_DOCS_URL = 'https://posthog.com/docs/data-warehouse/sources'; +/** Task type of the seeded step below, matched against the drain's result. */ +const WAREHOUSE_SEED_TASK_TYPE = 'warehouse'; + function resolveContinueUrl( sess: WizardSession, host: HostResolution, @@ -82,13 +85,19 @@ function warehouseSourceUrl( * one pass, and it is the only route offered once the list is too long to read. * * Returns undefined when nothing was detected, so the outro is unchanged for - * projects with no connectable source. + * projects with no connectable source — and when the run's own warehouse step + * connected them, where every bullet here would ask the user to redo work the + * wizard just did and send them at a new-source form that would collide with + * the source already created. */ function buildWarehouseNextSteps( sess: WizardSession, host: HostResolution, projectId: number | string, + completedSeededTypes: readonly string[], ): { heading: string; items: string[] } | undefined { + if (completedSeededTypes.includes(WAREHOUSE_SEED_TASK_TYPE)) return undefined; + const sources = getDetectedWarehouseSources(sess); if (sources.length === 0) return undefined; @@ -155,7 +164,7 @@ const warehouseSeedTasks: NonNullable = (sess) => { } return [ { - type: 'warehouse', + type: WAREHOUSE_SEED_TASK_TYPE, inputs: { sources: sources.map((s) => ({ kind: s.kind, @@ -401,6 +410,14 @@ ${warehouseReportInstruction(session)} } }, + buildOutroNextSteps: (sess, credentials, completedSeededTypes) => + buildWarehouseNextSteps( + sess, + credentials.host, + credentials.projectId, + completedSeededTypes, + ), + buildOutroData: (sess, credentials) => { const envVars = config.environment.getEnvVars( credentials.projectApiKey, @@ -426,10 +443,13 @@ ${warehouseReportInstruction(session)} changes, docsUrl: config.metadata.docsUrl, continueUrl, + // The linear sequence seeds no tasks, so nothing here was connected + // during the run. `buildOutroNextSteps` carries the orchestrated case. nextSteps: buildWarehouseNextSteps( sess, credentials.host, credentials.projectId, + [], ), // Set once the agent mirrors the report into a notebook and emits [NOTEBOOK_URL]. notebookUrl: sess.notebookUrl ?? undefined,