Skip to content

refactor(cli): call isCompatibilityActionable instead of mirroring the rule #48

Description

@mrgoonie

Problem

packages/cli/src/lib/skill-lookup.ts mirrors the compatibility actionability rule by hand:

export function isActionable(status: string | null | undefined): boolean {
  return status === 'declared' || status === 'verified';
}

packages/contracts owns the real rule (isCompatibilityActionable, with the vocabulary in COMPATIBILITY_STATUSES). Two implementations of one rule is exactly the drift the cross-surface parity suite in PR #41 exists to catch — and the CLI is the one surface that suite cannot reach.

Why it was not fixed in PR #41

Making the CLI import @skillx/contracts requires changing how the published artifact is bundled:

  • packages/cli/tsconfig.json sets rootDir: "./src", so any import outside src/ fails typecheck;
  • tsup treats dependencies as external by default, and @skillx/contracts ships TypeScript source with no compile step, so it has to go in devDependencies and be force-bundled (noExternal), together with the tsconfig paths entry that the bundler needs to resolve it.

Getting that wrong breaks the npm publish path, which release-please triggers automatically on merge to main. That is a poor trade for a one-line predicate.

Mitigation already in place

packages/contracts/src/compatibility.test.ts pins COMPATIBILITY_STATUSES to an explicit list, with a comment naming this mirror. Adding a sixth status now fails that test instead of silently leaving the CLI wrong.

What the fix needs

  1. Add @skillx/contracts to packages/cli devDependencies as workspace:*.
  2. Add the tsconfig paths entry and drop or widen rootDir.
  3. Add noExternal: ['@skillx/contracts'] to tsup.config.ts.
  4. Delete the mirrored predicate and delegate to the contract.
  5. Verify at the artifact level, not just in source: run the CLI bundler, then execute the resulting CLI (node bin/skillx.js check <slug> --target <runtime>) and confirm the bundle inlined the rule instead of leaving an unresolvable import.

Acceptance

  • The shipped CLI reports the same actionable decision as isCompatibilityActionable for all five statuses.
  • pnpm test still passes, and the parity test's note about the CLI limitation can be removed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions