Skip to content

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

Closed

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

  1. self-assigned this
    on Oct 19, 2026
  2. added and removed on Oct 19, 2026
  3. added 3 commits that reference this issue on Oct 20, 2026
    93f68d5
    815d29d
    ad16279
  4. added a commit that references this issue on Oct 20, 2026
    79f98fb
  5. mrgoonie commented on Oct 20, 2026

    @mrgoonie
    ContributorAuthor

    Shipped: fixed by #97 (squash 79f98fb, merged 2026-10-20 after isolated re-review CLEAN at 2be37c3). Deployed to production skillx-web (worker version 7d29ff0a). The CLI change ships to npm with the next release-please release.

  6. added
    shippedMerged/deployed
    and removed on Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requestshippedMerged/deployed

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions