Skip to content

refactor(apply): centralize plan result display and simplify grouped formatting #882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
toiroakr wants to merge 3 commits into
base: feature/changeset-grouping
Choose a base branch
from
Open

refactor(apply): centralize plan result display and simplify grouped formatting #882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42eb8a6
refactor(apply): simplify grouped display formatting with generic helper
toiroakr Apr 1, 2026
495a5ce
merge origin/feature/changeset-grouping and resolve conflicts
toiroakr Apr 1, 2026
615c666
refactor(apply): centralize plan result printing in apply.ts
toiroakr Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 61 additions & 3 deletions packages/sdk/src/cli/commands/apply/apply.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { describe, expect, test } from "vitest";
import { summarizePlanResultsForDisplay } from "./apply";
import { formatAuthHookChangeEntries } from "./auth";
import { buildPlannedExecutorsByName, formatExecutorChangeEntries } from "./executor";
import { formatResolverChangeEntries } from "./resolver";
import { formatTailorDBResourceChangeEntries } from "./tailordb";
import { formatWorkflowChangeEntries } from "./workflow";

type SummaryPlanResults = Parameters<typeof summarizePlanResultsForDisplay>[0];
type FunctionRegistryChangeSet = SummaryPlanResults["functionRegistry"]["changeSet"];
Expand Down Expand Up @@ -63,6 +68,32 @@ function createFixtureChangeSet<
} as unknown as T;
}

function computeDisplayEntries(results: SummaryPlanResults) {
return {
executorEntries: formatExecutorChangeEntries(
results.executor.changeSet,
buildPlannedExecutorsByName(results.executor.changeSet),
results.functionRegistry.executorFunctionChanges,
),
resolverEntries: formatResolverChangeEntries(
results.pipeline.changeSet.resolver,
results.functionRegistry.resolverFunctionChanges,
),
workflowEntries: formatWorkflowChangeEntries(
results.workflow.changeSet,
results.functionRegistry.workflowJobChanges,
),
authHookEntries: formatAuthHookChangeEntries(
results.auth.changeSet.authHook,
results.functionRegistry.authHookFunctionChanges,
),
tailorDBEntries: formatTailorDBResourceChangeEntries(
results.tailorDB.changeSet.type,
results.tailorDB.changeSet.gqlPermission,
),
};
}

describe("summarizePlanResultsForDisplay", () => {
test("counts grouped display entries instead of raw internal resources", () => {
const functionRegistry = createFixtureChangeSet<FunctionRegistryChangeSet>();
Expand Down Expand Up @@ -240,7 +271,16 @@ describe("summarizePlanResultsForDisplay", () => {
},
} satisfies SummaryPlanResults;

const summary = summarizePlanResultsForDisplay(results);
const { executorEntries, resolverEntries, workflowEntries, authHookEntries, tailorDBEntries } =
computeDisplayEntries(results);
const summary = summarizePlanResultsForDisplay(
results,
executorEntries,
resolverEntries,
workflowEntries,
authHookEntries,
tailorDBEntries,
);

expect(summary).toEqual({
create: 1,
Expand Down Expand Up @@ -379,7 +419,16 @@ describe("summarizePlanResultsForDisplay", () => {
},
} satisfies SummaryPlanResults;

const summary = summarizePlanResultsForDisplay(results);
const { executorEntries, resolverEntries, workflowEntries, authHookEntries, tailorDBEntries } =
computeDisplayEntries(results);
const summary = summarizePlanResultsForDisplay(
results,
executorEntries,
resolverEntries,
workflowEntries,
authHookEntries,
tailorDBEntries,
);

expect(summary).toEqual({
create: 0,
Expand Down Expand Up @@ -528,7 +577,16 @@ describe("summarizePlanResultsForDisplay", () => {
},
} satisfies SummaryPlanResults;

const summary = summarizePlanResultsForDisplay(results);
const { executorEntries, resolverEntries, workflowEntries, authHookEntries, tailorDBEntries } =
computeDisplayEntries(results);
const summary = summarizePlanResultsForDisplay(
results,
executorEntries,
resolverEntries,
workflowEntries,
authHookEntries,
tailorDBEntries,
);

expect(summary).toEqual({
create: 0,
Expand Down
139 changes: 91 additions & 48 deletions packages/sdk/src/cli/commands/apply/apply.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,22 @@ import {
applyFunctionRegistry,
collectFunctionEntries,
filterBundledWorkflowJobs,
isWorkflowJobFunctionName,
planFunctionRegistry,
splitFunctionRegistryChanges,
} from "./function-registry";
import {
formatChangeSetEntries,
printGroupedDisplaySection,
type GroupedDisplayEntry,
} from "./grouped-display";
import { applyIdP, planIdP } from "./idp";
import { buildMetaRequest, hasMatchingSdkVersion, sdkNameLabelKey } from "./label";
import { applyPipeline, formatResolverChangeEntries, planPipeline } from "./resolver";
import { applySecretManager, planSecretManager } from "./secret-manager";
import { applyStaticWebsite, planStaticWebsite } from "./staticwebsite";
import { applyTailorDB, formatTailorDBResourceChangeEntries, planTailorDB } from "./tailordb";
import { applyWorkflow, formatWorkflowChangeEntries, planWorkflow } from "./workflow";
import type { GroupedDisplayEntry } from "./grouped-display";
import type { OperatorClient } from "@/cli/shared/client";
import type { LoadedConfig } from "@/cli/shared/config-loader";

Expand Down Expand Up @@ -182,7 +187,7 @@ async function shouldForceApplyAll(
return false;
}

function printPlanSummary(results: {
function printPlanResults(results: {
functionRegistry: Awaited<ReturnType<typeof planFunctionRegistry>>;
tailorDB: Awaited<ReturnType<typeof planTailorDB>>;
staticWebsite: Awaited<ReturnType<typeof planStaticWebsite>>;
Expand All @@ -194,7 +199,55 @@ function printPlanSummary(results: {
workflow: Awaited<ReturnType<typeof planWorkflow>>;
secretManager: Awaited<ReturnType<typeof planSecretManager>>;
}) {
const summary = summarizePlanResultsForDisplay(results);
const executorEntries = formatExecutorChangeEntries(
results.executor.changeSet,
buildPlannedExecutorsByName(results.executor.changeSet),
results.functionRegistry.executorFunctionChanges,
);
const resolverEntries = formatResolverChangeEntries(
results.pipeline.changeSet.resolver,
results.functionRegistry.resolverFunctionChanges,
);
const workflowEntries = formatWorkflowChangeEntries(
results.workflow.changeSet,
results.functionRegistry.workflowJobChanges,
);
const authHookEntries = formatAuthHookChangeEntries(
results.auth.changeSet.authHook,
results.functionRegistry.authHookFunctionChanges,
);
const tailorDBEntries = formatTailorDBResourceChangeEntries(
results.tailorDB.changeSet.type,
results.tailorDB.changeSet.gqlPermission,
);
const authEntries: GroupedDisplayEntry[] = [
...formatChangeSetEntries(results.auth.changeSet.service, ["service"]),
...formatChangeSetEntries(results.auth.changeSet.idpConfig, ["idpConfig"]),
...formatChangeSetEntries(results.auth.changeSet.userProfileConfig, ["userProfileConfig"]),
...formatChangeSetEntries(results.auth.changeSet.tenantConfig, ["tenantConfig"]),
...formatChangeSetEntries(results.auth.changeSet.machineUser, ["machineUser"]),
...authHookEntries,
...formatChangeSetEntries(results.auth.changeSet.oauth2Client, ["oauth2Client"]),
...formatChangeSetEntries(results.auth.changeSet.scim, ["scimConfig"]),
...formatChangeSetEntries(results.auth.changeSet.scimResource, ["scimResource"]),
];

// Print grouped sections
printGroupedDisplaySection("Executors", executorEntries);
printGroupedDisplaySection("Workflows", workflowEntries);
printGroupedDisplaySection("TailorDB resources", tailorDBEntries);
printGroupedDisplaySection("Pipeline resolvers", resolverEntries);
printGroupedDisplaySection("Auth", authEntries);
Comment on lines +235 to +240
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 TailorDB services and Pipeline services no longer printed in dry-run output

The old code printed TailorDB service changes via serviceChangeSet.print() in planTailorDB and Pipeline service changes via serviceChangeSet.print() in planPipeline. Both calls were removed as part of the centralization, but the new printPlanResults function only prints grouped sections for "TailorDB resources" (types + gqlPermissions), "Pipeline resolvers", "Executors", "Workflows", and "Auth" — it never prints TailorDB service or Pipeline service changes. However, these change sets are still counted in the summary via summarizeChangeSets at apply.ts:393-394 and apply.ts:406. This means users will see a summary like "Plan: 3 to create..." where some counted changes (service creates/updates/deletes) are invisible in the output above the summary line.

Prompt for agents 
In printPlanResults (apply.ts), the grouped display sections printed at lines 236-240 are missing TailorDB services and Pipeline services. The old code printed these via serviceChangeSet.print() inside planTailorDB (tailordb/index.ts, removed at old line 1009) and planPipeline (resolver.ts, removed at old line 141).

To fix, either:
1. Add printGroupedDisplaySection calls for TailorDB services and Pipeline services in printPlanResults, e.g. using formatChangeSetEntries(results.tailorDB.changeSet.service, ["service"]) and formatChangeSetEntries(results.pipeline.changeSet.service, ["service"]) and printing them under appropriate section headers, OR
2. Fold the service entries into the existing grouped sections (e.g. prepend TailorDB service entries to the TailorDB resources section, and Pipeline service entries to the Pipeline resolvers section).

The key requirement is that these change sets are already counted in the summary at summarizeChangeSets (lines 392-410), so they must also be visible in the printed output for consistency.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


// Compute summary
const summary = summarizePlanResultsForDisplay(
results,
executorEntries,
resolverEntries,
workflowEntries,
authHookEntries,
tailorDBEntries,
);

logger.log(formatPlanSummary(summary));
}
Expand Down Expand Up @@ -299,20 +352,32 @@ function countUnchangedItemsWithoutChangedRelations<T extends HasName>(
* @param results.executor - Planned executor changes
* @param results.workflow - Planned workflow changes
* @param results.secretManager - Planned secret manager changes
* @param executorEntries - Pre-computed executor display entries
* @param resolverEntries - Pre-computed resolver display entries
* @param workflowEntries - Pre-computed workflow display entries
* @param authHookEntries - Pre-computed auth hook display entries
* @param tailorDBEntries - Pre-computed TailorDB display entries
* @returns Aggregated plan summary aligned with grouped display rows
*/
export function summarizePlanResultsForDisplay(results: {
functionRegistry: Awaited<ReturnType<typeof planFunctionRegistry>>;
tailorDB: Awaited<ReturnType<typeof planTailorDB>>;
staticWebsite: Awaited<ReturnType<typeof planStaticWebsite>>;
idp: Awaited<ReturnType<typeof planIdP>>;
auth: Awaited<ReturnType<typeof planAuth>>;
pipeline: Awaited<ReturnType<typeof planPipeline>>;
app: Awaited<ReturnType<typeof planApplication>>;
executor: Awaited<ReturnType<typeof planExecutor>>;
workflow: Awaited<ReturnType<typeof planWorkflow>>;
secretManager: Awaited<ReturnType<typeof planSecretManager>>;
}): PlanSummary {
export function summarizePlanResultsForDisplay(
results: {
functionRegistry: Awaited<ReturnType<typeof planFunctionRegistry>>;
tailorDB: Awaited<ReturnType<typeof planTailorDB>>;
staticWebsite: Awaited<ReturnType<typeof planStaticWebsite>>;
idp: Awaited<ReturnType<typeof planIdP>>;
auth: Awaited<ReturnType<typeof planAuth>>;
pipeline: Awaited<ReturnType<typeof planPipeline>>;
app: Awaited<ReturnType<typeof planApplication>>;
executor: Awaited<ReturnType<typeof planExecutor>>;
workflow: Awaited<ReturnType<typeof planWorkflow>>;
secretManager: Awaited<ReturnType<typeof planSecretManager>>;
},
executorEntries: ReadonlyArray<GroupedDisplayEntry>,
resolverEntries: ReadonlyArray<GroupedDisplayEntry>,
workflowEntries: ReadonlyArray<GroupedDisplayEntry>,
authHookEntries: ReadonlyArray<GroupedDisplayEntry>,
tailorDBEntries: ReadonlyArray<GroupedDisplayEntry>,
): PlanSummary {
const summary: PlanSummary = {
create: 0,
update: 0,
Expand Down Expand Up @@ -348,10 +413,7 @@ export function summarizePlanResultsForDisplay(results: {
addPlanSummary(
summary,
summarizeDisplayEntries(
formatTailorDBResourceChangeEntries(
results.tailorDB.changeSet.type,
results.tailorDB.changeSet.gqlPermission,
),
tailorDBEntries,
countUnchangedNamesExcludingChanged(
[
results.tailorDB.changeSet.type.unchanged,
Expand All @@ -374,10 +436,7 @@ export function summarizePlanResultsForDisplay(results: {
addPlanSummary(
summary,
summarizeDisplayEntries(
formatResolverChangeEntries(
results.pipeline.changeSet.resolver,
results.functionRegistry.resolverFunctionChanges,
),
resolverEntries,
countUnchangedItemsWithoutChangedRelations(
results.pipeline.changeSet.resolver.unchanged as ReadonlyArray<
HasName & { namespaceName?: string }
Expand All @@ -397,11 +456,7 @@ export function summarizePlanResultsForDisplay(results: {
addPlanSummary(
summary,
summarizeDisplayEntries(
formatExecutorChangeEntries(
results.executor.changeSet,
buildPlannedExecutorsByName(results.executor.changeSet),
results.functionRegistry.executorFunctionChanges,
),
executorEntries,
countUnchangedItemsWithoutChangedRelations(
results.executor.changeSet.unchanged,
[
Expand All @@ -418,10 +473,7 @@ export function summarizePlanResultsForDisplay(results: {
addPlanSummary(
summary,
summarizeDisplayEntries(
formatWorkflowChangeEntries(
results.workflow.changeSet,
results.functionRegistry.workflowJobChanges,
),
workflowEntries,
countUnchangedItemsWithoutChangedRelations(
results.workflow.changeSet.unchanged as ReadonlyArray<
HasName & { usedJobNames?: string[] }
Expand All @@ -440,10 +492,7 @@ export function summarizePlanResultsForDisplay(results: {
addPlanSummary(
summary,
summarizeDisplayEntries(
formatAuthHookChangeEntries(
results.auth.changeSet.authHook,
results.functionRegistry.authHookFunctionChanges,
),
authHookEntries,
countUnchangedItemsWithoutChangedRelations(
results.auth.changeSet.authHook.unchanged,
[
Expand Down Expand Up @@ -627,23 +676,18 @@ export async function apply(options?: ApplyOptions) {
);
const unchangedWorkflowJobs = new Set(
functionRegistry.changeSet.unchanged
.map((entry) => entry.name)
.filter((name) => name.startsWith("workflow--"))
.map((name) => name.slice("workflow--".length)),
.filter((entry) => isWorkflowJobFunctionName(entry.name))
.map((entry) => entry.name.slice("workflow--".length)),
);
const [tailorDB, staticWebsite, idp, auth, pipeline, app, executor, workflow, secretManager] =
await Promise.all([
withSpan("plan.tailorDB", () => planTailorDB(ctx)),
withSpan("plan.staticWebsite", () => planStaticWebsite(ctx)),
withSpan("plan.idp", () => planIdP(ctx)),
withSpan("plan.auth", () => planAuth(ctx, functionRegistry.authHookFunctionChanges)),
withSpan("plan.pipeline", () =>
planPipeline(ctx, functionRegistry.resolverFunctionChanges),
),
withSpan("plan.auth", () => planAuth(ctx)),
withSpan("plan.pipeline", () => planPipeline(ctx)),
withSpan("plan.application", () => planApplication(ctx)),
withSpan("plan.executor", () =>
planExecutor(ctx, functionRegistry.executorFunctionChanges),
),
withSpan("plan.executor", () => planExecutor(ctx)),
withSpan("plan.workflow", () =>
planWorkflow(
client,
Expand All @@ -652,7 +696,6 @@ export async function apply(options?: ApplyOptions) {
workflowService?.workflows ?? {},
workflowBuildResult?.mainJobDeps ?? {},
unchangedWorkflowJobs,
functionRegistry.workflowJobChanges,
),
),
withSpan("plan.secretManager", () => planSecretManager(ctx)),
Expand Down Expand Up @@ -763,7 +806,7 @@ export async function apply(options?: ApplyOptions) {
}
});

printPlanSummary({
printPlanResults({
functionRegistry,
tailorDB,
staticWebsite,
Expand Down
16 changes: 0 additions & 16 deletions packages/sdk/src/cli/commands/apply/auth.plan.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
AuthOAuth2Client_GrantType,
} from "@tailor-proto/tailor/v1/auth_resource_pb";
import { describe, expect, test, vi } from "vitest";
import { logger } from "@/cli/shared/logger";
import { formatAuthHookChangeEntries, planAuth } from "./auth";
import type { PlanContext } from "./apply";
import type { Application } from "@/cli/services/application";
Expand Down Expand Up @@ -480,21 +479,6 @@ describe("planAuth", () => {
expect(result.changeSet.idpConfig.updates[0]?.name).toBe("default");
expect(result.changeSet.idpConfig.unchanged).toHaveLength(0);
});

test("prints auth child resources under a single flat Auth section", async () => {
const client = createMockClient();
const logSpy = vi.spyOn(logger, "log").mockImplementation(() => {});

await planAuth(createContext(client));

expect(logSpy.mock.calls.map(([message]) => message)).toEqual([
"Auth:",
" + auth-a (service)",
" + manager-machine-user (machineUser)",
" + auth-a/before-login (authHook)",
" + sample (oauth2Client)",
]);
});
});

describe("formatAuthHookChangeEntries", () => {
Expand Down
Loading
Loading