feat(builder): contract labels

[dbeal-eth]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 75a3f4f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[saturn-dbeal]

Review: feat(builder): contract labels

Built and ran the test suite — 23 tests failing across 7 suites. The core feature (contract labels) looks good architecturally, and the schema cleanup is a nice improvement. But there are a few categories of issues to address before merge.

1. Tests need updating for labels: undefined in outputs (13 failures)

Every step that now stores config.labels in its output will emit labels: undefined when no labels are provided. Tests use toStrictEqual which catches this. Either:

• Omit labels from output when undefined (e.g. ...(config.labels && { labels: config.labels }))
• Or update all test expectations to include labels: undefined

The first approach is cleaner — it avoids polluting serialized state with undefined fields.

Affected: deploy.test.ts, invoke.test.ts, pull.test.ts, clone.test.ts, builder.test.ts

2. .strict().merge(sharedActionSchema) breaks strict validation (4 failures)

The pattern:

.strict()
.merge(sharedActionSchema);

In Zod, .merge() after .strict() creates a new schema that no longer enforces strict mode. This means schemas silently accept unknown keys — the "fails when setting invalid value" tests all fail because { invalid: ["something"] } no longer throws.

Fix: apply .strict() after the merge, or compose differently:

.merge(sharedActionSchema)
.strict();

Affected: deploy.test.ts, invoke.test.ts, pull.test.ts, clone.test.ts

3. artifactNameRegex is now stricter — breaks test fixtures (3 failures)

The new regex ^[A-Z]{1}[\w]+$ requires artifacts to start with uppercase. The old .refine() was more lenient. Test fixtures use lowercase artifact names like wohoo and greeter which now fail validation.

This is actually a reasonable tightening of validation since Solidity contracts do conventionally start with uppercase, but:

• The test data in definition.test.ts uses artifact: "wohoo" — needs to be "Wohoo" or similar
• The builder.test.ts fixtures need the same treatment

4. Error message strings changed (2 failures)

Tests check for specific error substrings like "Package name exceeds 32 bytes" and "Package version exceeds 32 bytes" — the new regex-based validation produces different messages. Update test expectations to match the new messages.

5. varSchema lost fields

The old varSchema explicitly had defaultValue, description, and depends. The new version:

export const varSchema = z.object({}).catchall(z.string());

uses catchall which accepts any string values, so description still works as a string. But depends (an array) would be rejected. Since sharedActionSchema is not merged here, var actions lose depends support. If var actions should support depends, merge the shared schema.

6. LSP src/index.ts looks like boilerplate

The new packages/lsp/src/index.ts appears to be the VS Code LSP example server nearly verbatim — it validates uppercase words and has placeholder completions (TypeScript/JavaScript). Presumably this is scaffolding for future work? If so, consider noting that in the PR description or a TODO comment. If it's meant to be functional, it needs the Cannon-specific logic.

7. Minor: include field added without schema validation

chainDefinitionSchema now has:

include: z.array(z.string()).describe("...").optional(),

This is fine, but there's no corresponding runtime handling visible in this diff. If it's for a future PR, maybe note that.

---

Summary: The labels feature and schema cleanup are solid improvements. The main work needed is fixing the test suite — mostly mechanical updates. The .strict().merge() ordering is the most important semantic bug to fix.