Issue #660 Add --accept-license-terms flag to publish and sync

[podthesquire]

Fix for issue #660

Summary

• Adds --accept-license-terms flag to the publish and sync CLI commands
• When the flag is set, acceptLicenseTerms: true is sent in the JSON payload; otherwise false
• Replaces the previously hardcoded acceptLicenseTerms: true in cmdPublish

Changes

• cli.ts: Added --accept-license-terms option to publish and sync command definitions; sync passes the flag value through to cmdSync
• syncTypes.ts: Added acceptLicenseTerms?: boolean to SyncOptions
• sync.ts: Forwards options.acceptLicenseTerms to cmdPublish
• publish.ts: Added acceptLicenseTerms?: boolean to options type; replaced hardcoded true with options.acceptLicenseTerms === true

Test plan

• clawhub publish --accept-license-terms sends acceptLicenseTerms: true in payload
• clawhub publish (without flag) sends acceptLicenseTerms: false
• clawhub sync --accept-license-terms propagates true to each cmdPublish call
• clawhub sync (without flag) propagates false to each cmdPublish call
• clawhub publish --help and clawhub sync --help show the new flag

[vercel]

@podthesquire is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Greptile Summary

This PR adds an --accept-license-terms CLI flag to the publish and sync commands, replacing a previously hardcoded acceptLicenseTerms: true in the publish payload. The implementation is straightforward and correct: Commander.js correctly maps --accept-license-terms to options.acceptLicenseTerms, and options.acceptLicenseTerms === true safely coerces the absent (undefined) case to false.

Key observations:

• The logic and data flow across cli.ts → sync.ts / publish.ts is correct and complete.
• One test regression: The existing test in publish.test.ts calls cmdPublish without passing acceptLicenseTerms: true, then asserts expect(payload.acceptLicenseTerms).toBe(true) — an assertion that passed only because the value was previously hardcoded. With this change it will produce false and the test will fail. The test needs to either pass the flag explicitly or update the assertion.
• No new tests were added to cover the new --accept-license-terms flag behaviour (e.g. verifying that omitting the flag sends false).

Confidence Score: 3/5

• The production code is correct, but a pre-existing test will break due to an unupdated assertion — fix required before merging.
• The logic change itself is sound. However, the hardcoded true removal invalidates an existing test assertion in publish.test.ts (line 90) that was never updated to account for the new optional flag. This will cause the test suite to fail and must be fixed before the PR can be merged safely.
• packages/clawdhub/src/cli/commands/publish.test.ts — existing test assertion at line 90 needs to be updated to reflect that acceptLicenseTerms is no longer hardcoded.

Comments Outside Diff (1)

1. packages/clawdhub/src/cli/commands/publish.test.ts, line 69-90 (link)

   Test will now fail after this change

   The existing test calls cmdPublish without passing acceptLicenseTerms in the options object, then asserts expect(payload.acceptLicenseTerms).toBe(true) on line 90.

   Before this PR, acceptLicenseTerms was hardcoded to true in publish.ts, so the assertion passed regardless. Now that the code evaluates options.acceptLicenseTerms === true, and options.acceptLicenseTerms is undefined in this test call, the payload will contain acceptLicenseTerms: false — causing the assertion to fail.

   The test needs to either:

   1. Pass acceptLicenseTerms: true in the options (to test the opt-in path), or
   2. Update the assertion to toBe(false) (to document the default-off behaviour)

   Given the PR intent, option 1 is more appropriate here, and a separate test case for the default false path would improve coverage:

_{Last reviewed commit: 3ce4068}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ce40686c9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Preserve default license acceptance for publish payload

This change makes cmdPublish send acceptLicenseTerms: false whenever --accept-license-terms is omitted, which breaks existing clawhub publish/clawhub sync workflows that previously succeeded without that flag. The server-side publish handler rejects any payload where this field is not true (convex/httpApiV1/skillsV1.ts checks payload.acceptLicenseTerms !== true), so this introduces a runtime 400 regression for callers that do not add the new option.

Useful? React with 👍 / 👎.

[podthesquire]

Clawhub implemented a breaking change requiring this field be set for the API calls. Defaulting to false maintains the current intended behavior--preventing users from submitting without accepting license terms.

[vercel]

Additional Suggestion:

Test expects acceptLicenseTerms to be true but receives false after PR changed the behavior to require explicit opt-in

[claytantor]

I had to update my local CLI implementation to work around this.

[zeuslabsllc]

Thanks for taking this on — I reviewed the current state from a live publisher perspective.

My recommendation is:

1. Fix the tests before merge

   • the review feedback about the existing publish test breaking looks valid
   • this PR also needs explicit coverage for both paths:
     • --accept-license-terms present -> payload sends true
     • flag omitted -> payload sends false (or whatever default behavior is intended)

2. Reconsider the UX/default behavior

   • as written, this fixes the transport bug but still leaves normal clawhub publish failing unless users discover the new flag
   • that may be technically correct, but it is still a rough publish UX for the common path

3. Clarify intended product semantics

   • if the registry truly requires explicit acceptance every time, the flag approach makes sense
   • if the product intent is “publish should work once the user accepts MIT-0 terms for this publish flow”, then the CLI may want a smoother default/prompt path instead of fail-by-default

We independently reproduced this on clawhub@0.7.0, and the current workaround is successful direct publish with acceptLicenseTerms: true in the payload.

So from my side:

• yes, this PR is worth having
• but it should not merge without the test fix + a deliberate UX decision

[ngutman]

Closing this as not needed.

Issue #660 is a real problem for the published npm CLI, but this PR is fixing it in the wrong direction.

Current server behavior requires acceptLicenseTerms === true on publish, and current main already handles that by always sending acceptLicenseTerms: true from the CLI. That change landed earlier in 2687d67 (feat: enforce MIT-0 skill licensing).

This PR changes the behavior from:

• publish/sync always send acceptLicenseTerms: true

to:

• publish/sync send false unless the user discovers and passes --accept-license-terms

That would reintroduce the publish failure on the default CLI path, which is the opposite of what issue #660 needs.

I also verified that the existing publish test passes on current main and fails on this PR because acceptLicenseTerms flips from true to false.

So the underlying issue here is a release gap, not a missing flag:

• npm clawhub@0.7.0 was published before the CLI fix landed
• what’s needed is a new CLI release from current main, not this behavior change

Thanks for digging into the bug, but there isn’t anything here we should merge.

[podthesquire]

It's odd that you would have a legal contract, but have that automatically accepted as part of the CLI use without the user knowing. That wouldn't hold up in court.

On the other hand, I get not wanting to break workflows for all of your users. But that's where we were, so... ¯_(ツ)_/¯