Skip to content

Dry run before publishing #85

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
mabadir wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Dry run before publishing #85

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e3c5d41
dry run test
mabadir Jul 31, 2026
1910f24
Merge branch 'main' into ma/publish-dry-run
mabadir Aug 3, 2026
7e0998f
Publish new definitions privately until first version succeeds
mabadir Aug 4, 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
4 changes: 2 additions & 2 deletions .github/workflows/module-definitions.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ jobs:
- name: Generate publish dry-run plan
id: publish_plan
continue-on-error: true
run: node tools/ravion-modules/dist/src/cli.js publish --format markdown --output publish-plan.md 2> publish-plan.err
run: node tools/ravion-modules/dist/src/cli.js publish --validate-remote --format markdown --output publish-plan.md 2> publish-plan.err

- name: Comment publish dry-run plan
uses: actions/github-script@v7
Expand Down Expand Up @@ -136,7 +136,7 @@ jobs:
working-directory: tools/ravion-modules

- name: Dry-run publish plan
run: node tools/ravion-modules/dist/src/cli.js publish
run: node tools/ravion-modules/dist/src/cli.js publish --validate-remote

- name: Configure Git tag author
run: |
Expand Down
2 changes: 1 addition & 1 deletion tools/ravion-modules/src/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ if (command === "validate") {
const outputPath = getArgValue(args, "--output");
let result;
try {
result = await publishDefinitions(compiled, client, { dryRun: localDev ? args.includes("--dry-run") : !args.includes("--apply"), localDev, localDevForce: args.includes("--force"), localDevSourceRef, logger: (message) => console.error(`[publish] ${message}`) });
result = await publishDefinitions(compiled, client, { dryRun: localDev ? args.includes("--dry-run") : !args.includes("--apply"), validateRemote: args.includes("--validate-remote"), localDev, localDevForce: args.includes("--force"), localDevSourceRef, logger: (message) => console.error(`[publish] ${message}`) });
} catch (error) {
if (isPublishPlanError(error)) {
const output = format === "markdown" ? formatPublishPlanMarkdown(error.result) : JSON.stringify(error.result, null, 2);
Expand Down
83 changes: 82 additions & 1 deletion tools/ravion-modules/src/publish.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export interface PublishResult {
dryRun: boolean;
items: PublishPlanItem[];
errors?: PublishPlanErrorItem[];
validationErrors?: PublishValidationErrorItem[];
}

export interface PublishPlanErrorItem {
Expand All @@ -38,6 +39,12 @@ export interface PublishPlanErrorItem {
diff?: string;
}

export interface PublishValidationErrorItem {
type: string;
version: string;
message: string;
}

export interface ModuleDefinitionInput {
type: string;
name: string;
Expand Down Expand Up @@ -66,6 +73,7 @@ export interface RavionApiClientOptions {

export interface PublishOptions {
dryRun?: boolean;
validateRemote?: boolean;
localDev?: boolean;
localDevForce?: boolean;
localDevSourceRef?: string;
Expand All @@ -80,6 +88,7 @@ export interface RavionModuleApiClient {
patchModuleDefinition(input: ModuleDefinitionPatchInput): Promise<RemoteModuleDefinition>;
listModuleVersions(moduleDefinitionId: string): Promise<RemoteModuleVersion[]>;
createModuleVersion(input: ModuleVersionInput): Promise<RemoteModuleVersion>;
validateModuleVersion(input: ModuleVersionInput): Promise<void>;
}

export class PublishError extends Error {
Expand Down Expand Up @@ -127,6 +136,7 @@ export async function publishDefinitions(
inventory.definitions.map((definition) => [definition.type, definition]),
);
const items: PublishPlanItem[] = [];
const validationErrors: PublishValidationErrorItem[] = [];

for (const definition of [...definitionsToPublish].sort((left, right) =>
left.type.localeCompare(right.type),
Expand Down Expand Up @@ -225,6 +235,31 @@ export async function publishDefinitions(
latestRemoteVersion?.version,
),
);
if (dryRun && options.validateRemote) {
if (!remoteDefinition) {
options.logger?.(
`Skipping remote validation for ${definition.type}@${definition.version}; the module definition does not exist remotely yet.`,
);
Comment on lines +265 to +269

@greptile-apps greptile-apps Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 New definitions bypass validation

When a new module has config accepted locally but rejected by the Ravion API, both dry runs skip remote validation because the definition does not exist yet. The later --apply run creates and globally publishes the definition before version creation fails, leaving a published definition with no version and failing the main-branch publish workflow.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tools/ravion-modules/src/publish.ts
Line: 238-242

Comment:
**New definitions bypass validation**

When a new module has config accepted locally but rejected by the Ravion API, both dry runs skip remote validation because the definition does not exist yet. The later `--apply` run creates and globally publishes the definition before version creation fails, leaving a published definition with no version and failing the main-branch publish workflow.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

} else {
try {
await client.validateModuleVersion({
moduleDefinitionId: remoteDefinition.id,
version: definition.version,
description: definition.releaseDescription,
config: definition.module,
});
options.logger?.(
`Remote validation passed for ${definition.type}@${definition.version}.`,
);
} catch (error) {
validationErrors.push({
type: definition.type,
version: definition.version,
message: formatUnknownError(error),
});
}
}
}
if (!dryRun) {
if (!remoteDefinition) {
throw new PublishError(
Expand All @@ -235,6 +270,13 @@ export async function publishDefinitions(
}
}

if (validationErrors.length > 0) {
throw new PublishPlanError(
`Remote validation failed for ${validationErrors.length} module version(s).`,
{ dryRun, items, validationErrors },
);
}

return { dryRun, items };
}

Expand Down Expand Up @@ -295,6 +337,24 @@ export function formatPublishPlanMarkdown(result: PublishResult): string {
"",
];

if (result.validationErrors && result.validationErrors.length > 0) {
lines.push(
"### 🚨 Remote Validation Failures 🚨",
"",
"The Ravion API rejected these module version configs during dry-run validation. Fix the module definition config before merging.",
"",
"| Module | Release Version | Error |",
"| --- | --- | --- |",
);
for (const error of result.validationErrors) {
lines.push(
`| \`${escapeMarkdownTableCell(error.type)}\` | \`${escapeMarkdownTableCell(error.version)}\` | ${escapeMarkdownTableCell(error.message)} |`,
);
}
lines.push("");
return lines.join("\n");

@greptile-apps greptile-apps Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Validation errors hide plan items

When one module fails remote validation, the result still contains planned changes for every processed definition, but this early return renders only the failure table. The workflow uses this Markdown directly for the PR comment, so reviewers cannot see the other valid releases or their diffs.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tools/ravion-modules/src/publish.ts
Line: 340-355

Comment:
**Validation errors hide plan items**

When one module fails remote validation, the result still contains planned changes for every processed definition, but this early return renders only the failure table. The workflow uses this Markdown directly for the PR comment, so reviewers cannot see the other valid releases or their diffs.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

}

if (result.errors && result.errors.length > 0) {
lines.push(
"### 🚨 Release Config Conflicts 🚨",
Expand Down Expand Up @@ -771,6 +831,17 @@ class HttpRavionModuleApiClient implements RavionModuleApiClient {
return this.call<RemoteModuleVersion>("POST", "/module-versions", { data: input });
}

async validateModuleVersion(input: ModuleVersionInput): Promise<void> {
const { status } = await this.request("POST", "/module-versions", {
data: { ...input, dryRun: true },
});
if (status !== 202) {
throw new PublishError(
`POST /module-versions dry-run validation returned HTTP ${status} instead of 202. The Ravion API may not support dryRun yet, and the module version may have been created for real.`,
);
}
}

private async list<T>(path: string, query: Record<string, string> = {}): Promise<T[]> {
const items: T[] = [];
let cursor: string | undefined;
Expand All @@ -797,6 +868,16 @@ class HttpRavionModuleApiClient implements RavionModuleApiClient {
query: Record<string, string> = {},
options: { unwrap?: boolean } = {},
): Promise<T> {
const { payload } = await this.request(method, path, input, query);
return (options.unwrap === false ? payload : unwrapApiPayload(payload)) as T;
}

private async request(
method: string,
path: string,
input?: unknown,
query: Record<string, string> = {},
): Promise<{ status: number; payload: unknown }> {
const headers: Record<string, string> = { "Content-Type": "application/json" };
if (this.token) {
headers.Authorization = `Bearer ${this.token}`;
Expand All @@ -823,7 +904,7 @@ class HttpRavionModuleApiClient implements RavionModuleApiClient {
`${method} ${url} failed with HTTP ${response.status}: ${message}\nResponse body:\n${formatResponsePayload(payload)}`,
);
}
return (options.unwrap === false ? payload : unwrapApiPayload(payload)) as T;
return { status: response.status, payload };
}
}

Expand Down
67 changes: 67 additions & 0 deletions tools/ravion-modules/test/publish.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,64 @@ describe("publish", () => {
assert.equal(client.createdVersions.length, 0);
});

it("validates create-version items remotely during dry run when requested", async () => {
const client = new MockRavionClient({
definitions: [{ id: "vpc", type: "ravion-aws-vpc", name: "AWS VPC", description: "AWS VPC and subnets." }],
});

const result = await publishDefinitions([createCompiledDefinition()], client, { dryRun: true, validateRemote: true });

assert.deepEqual(result.items.map(({ action, dryRun }) => ({ action, dryRun })), [{ action: "create-version", dryRun: true }]);
assert.deepEqual(client.validatedVersions, [
{ moduleDefinitionId: "vpc", version: "1.2.3", description: "Add subnet options.", config: { inputs: [{ id: "name", type: "string", label: "Name" }] } },
]);
assert.equal(client.createdVersions.length, 0);
});
Comment on lines +161 to +173

@greptile-apps greptile-apps Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 HTTP validation path remains untested

This test uses MockRavionClient, so it does not exercise the changed HTTP request envelope or the strict 202 response check in HttpRavionModuleApiClient.validateModuleVersion. Add an HTTP-client test for the dry-run request and success status so a mismatch with the API contract does not make every validation workflow fail.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: tools/ravion-modules/test/publish.test.ts
Line: 56-68

Comment:
**HTTP validation path remains untested**

This test uses `MockRavionClient`, so it does not exercise the changed HTTP request envelope or the strict `202` response check in `HttpRavionModuleApiClient.validateModuleVersion`. Add an HTTP-client test for the dry-run request and success status so a mismatch with the API contract does not make every validation workflow fail.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


it("fails the dry run with validation details when the API rejects a config", async () => {
const client = new MockRavionClient({
definitions: [{ id: "vpc", type: "ravion-aws-vpc", name: "AWS VPC", description: "AWS VPC and subnets." }],
});
client.onValidateVersion = async () => {
throw new Error('POST /module-versions failed with HTTP 422: inputs.0.required: field is not allowed');
};

await assert.rejects(
publishDefinitions([createCompiledDefinition()], client, { dryRun: true, validateRemote: true }),
(error: unknown) => {
assert.ok(error instanceof PublishPlanError);
assert.deepEqual(error.result.validationErrors, [
{ type: "ravion-aws-vpc", version: "1.2.3", message: "Error: POST /module-versions failed with HTTP 422: inputs.0.required: field is not allowed" },
]);
const markdown = formatPublishPlanMarkdown(error.result);
assert.match(markdown, /Remote Validation Failures/);
assert.match(markdown, /field is not allowed/);
return true;
},
);
assert.equal(client.createdVersions.length, 0);
});

it("skips remote validation for definitions that do not exist remotely yet", async () => {
const client = new MockRavionClient();

const result = await publishDefinitions([createCompiledDefinition()], client, { dryRun: true, validateRemote: true });

assert.deepEqual(result.items.map(({ action }) => action), ["create-definition", "create-version"]);
assert.equal(client.validatedVersions.length, 0);
assert.equal(client.createdVersions.length, 0);
});

it("does not validate remotely during dry run when not requested", async () => {
const client = new MockRavionClient({
definitions: [{ id: "vpc", type: "ravion-aws-vpc", name: "AWS VPC", description: "AWS VPC and subnets." }],
});

await publishDefinitions([createCompiledDefinition()], client, { dryRun: true });

assert.equal(client.validatedVersions.length, 0);
});

it("fails with structured conflict details when an existing version has different config", async () => {
const remoteConfig = {
inputs: [
Expand Down Expand Up @@ -439,8 +497,10 @@ class MockRavionClient implements RavionModuleApiClient {
createdDefinitions: ModuleDefinitionInput[] = [];
patchedDefinitions: ModuleDefinitionPatchInput[] = [];
createdVersions: ModuleVersionInput[] = [];
validatedVersions: ModuleVersionInput[] = [];
onListModuleVersions?: (moduleDefinitionId: string) => Promise<RemoteModuleVersion[]>;
onCreateVersion?: (input: ModuleVersionInput) => Promise<void>;
onValidateVersion?: (input: ModuleVersionInput) => Promise<void>;

constructor(options: { definitions?: RemoteModuleDefinition[]; versionsByDefinitionId?: Record<string, RemoteModuleVersion[]> } = {}) {
this.definitions = options.definitions ?? [];
Expand Down Expand Up @@ -485,4 +545,11 @@ class MockRavionClient implements RavionModuleApiClient {
this.versionsByDefinitionId[input.moduleDefinitionId] = [...(this.versionsByDefinitionId[input.moduleDefinitionId] ?? []), version];
return version;
}

async validateModuleVersion(input: ModuleVersionInput): Promise<void> {
if (this.onValidateVersion) {
await this.onValidateVersion(input);
}
this.validatedVersions.push(input);
}
}
3 changes: 2 additions & 1 deletion tools/ravion-modules/test/workflows.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ test("module definition workflow has valid syntax and expected jobs", async () =
assert.deepEqual(jobs.publish.permissions, { contents: "write" });
assert.ok(jobs.validate.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js guardrails"));
assert.ok(jobs.validate.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js compile"));
assert.ok(jobs["publish-plan"].steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --format markdown --output publish-plan.md 2> publish-plan.err"));
assert.ok(jobs["publish-plan"].steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --validate-remote --format markdown --output publish-plan.md 2> publish-plan.err"));
assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --validate-remote"));
assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js tags --api --create --overwrite > release-tags.json"));
assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js push-tags --plan release-tags.json"));
assert.ok(jobs.publish.steps.some((step) => step.run === "node tools/ravion-modules/dist/src/cli.js publish --apply"));
Expand Down
Loading